Re: [petsc-dev] Hijacked MPI calls...

2016-09-22 Thread Satish Balay
merged the fix to 'next'

https://bitbucket.org/petsc/petsc/commits/1dac896b5164bd7681661a63f944dd271192ecc1

Satish

On Thu, 15 Sep 2016, Barry Smith wrote:

> 
> > On Sep 15, 2016, at 11:06 PM, Jed Brown  wrote:
> > 
> > Satish Balay  writes:
> > 
> >> On Tue, 13 Sep 2016, Satish Balay wrote:
> >> 
> >>> On Tue, 13 Sep 2016, Satish Balay wrote:
> >>> 
>  And this one is perhaps for Matt..
> >>> 
> >>> Ops - perhaps Jed needs to look at this..
>  
> >>> 
>   CC arch-clang-cmplx/obj/src/dm/impls/composite/pack.o
>  /home/balay/petsc/src/dm/impls/composite/pack.c:657:17: warning: passing 
>  an object that undergoes default argument promotion to 'va_start' has 
>  undefined behavior [-Wvarargs]
>   va_start(Argp,imode);
> ^
>  /home/balay/petsc/src/dm/impls/composite/pack.c:641:61: note: parameter 
>  of type 'InsertMode' is declared here
>  PetscErrorCode  DMCompositeGather(DM dm,Vec gvec,InsertMode imode,...)
> ^
>  1 warning generated.
>  <
> >> 
> >> The fix here is:
> >> 
> >> -PetscErrorCode  DMCompositeGather(DM dm,Vec gvec,InsertMode imode,...)
> >> +PetscErrorCode  DMCompositeGather(DM dm,InsertMode imode,Vec gvec,...)
> >> 
> >> [Barry is ok - but Jed,Matt might object?]
> > 
> > It's annoying, but it appears to be needed.  It's screwy because enum
> > ought to be handled no differently than int for this purpose.
> 
>Yeah, we checked and sizeof(enum type) = 4 so there is no need for 
> promotion so I blame it on compiler flakiness.
> 
> 



Re: [petsc-dev] Hijacked MPI calls...

2016-09-16 Thread Jed Brown
Barry Smith  writes:

>> Well, the size of enums in general is compiler-dependent.
>
>   Sure, but with THIS compiler it is 4 hence will not be promoted. So the 
> compiler is being obnoxious complaining about code that might have 
> difficulties with other compilers IMHO.

We usually appreciate it when compilers call out non-portable code.

>>  (I think our
>> use of PetscEnum is technically incorrect unless we have already ensured
>> that the actual enums have that size.)  But even if the size varies by
>> enum, it's still well-defined and behaves like the compatible int as far
>> as varargs should be concerned.
>
>   Interestingly I found somewhere on the web that anything smaller than int 
> is always promoted to int size when passed through ... in the same way float 
> is promoted to double.  So if the enum had size 2 it would be promoted to 
> size 4 when passed through ... and thus the warning from the compiler would 
> be reasonable, IMHO.  Just a random page that discusses the promotion 
> https://www.eskimo.com/~scs/cclass/int/sx11c.html

Yeah, good point.  We could force the enum to be 4 bytes, but I doubt
that would fix the warning.


signature.asc
Description: PGP signature


Re: [petsc-dev] Hijacked MPI calls...

2016-09-15 Thread Barry Smith

> On Sep 15, 2016, at 11:20 PM, Jed Brown  wrote:
> 
> Barry Smith  writes:
>>> It's annoying, but it appears to be needed.  It's screwy because enum
>>> ought to be handled no differently than int for this purpose.
>> 
>>   Yeah, we checked and sizeof(enum type) = 4 so there is no need for 
>> promotion so I blame it on compiler flakiness.
> 
> Well, the size of enums in general is compiler-dependent.

  Sure, but with THIS compiler it is 4 hence will not be promoted. So the 
compiler is being obnoxious complaining about code that might have difficulties 
with other compilers IMHO.

>  (I think our
> use of PetscEnum is technically incorrect unless we have already ensured
> that the actual enums have that size.)  But even if the size varies by
> enum, it's still well-defined and behaves like the compatible int as far
> as varargs should be concerned.

  Interestingly I found somewhere on the web that anything smaller than int is 
always promoted to int size when passed through ... in the same way float is 
promoted to double.  So if the enum had size 2 it would be promoted to size 4 
when passed through ... and thus the warning from the compiler would be 
reasonable, IMHO.  Just a random page that discusses the promotion 
https://www.eskimo.com/~scs/cclass/int/sx11c.html






Re: [petsc-dev] Hijacked MPI calls...

2016-09-15 Thread Jed Brown
Barry Smith  writes:
>> It's annoying, but it appears to be needed.  It's screwy because enum
>> ought to be handled no differently than int for this purpose.
>
>Yeah, we checked and sizeof(enum type) = 4 so there is no need for 
> promotion so I blame it on compiler flakiness.

Well, the size of enums in general is compiler-dependent.  (I think our
use of PetscEnum is technically incorrect unless we have already ensured
that the actual enums have that size.)  But even if the size varies by
enum, it's still well-defined and behaves like the compatible int as far
as varargs should be concerned.


signature.asc
Description: PGP signature


Re: [petsc-dev] Hijacked MPI calls...

2016-09-15 Thread Barry Smith

> On Sep 15, 2016, at 11:06 PM, Jed Brown  wrote:
> 
> Satish Balay  writes:
> 
>> On Tue, 13 Sep 2016, Satish Balay wrote:
>> 
>>> On Tue, 13 Sep 2016, Satish Balay wrote:
>>> 
 And this one is perhaps for Matt..
>>> 
>>> Ops - perhaps Jed needs to look at this..
 
>>> 
  CC arch-clang-cmplx/obj/src/dm/impls/composite/pack.o
 /home/balay/petsc/src/dm/impls/composite/pack.c:657:17: warning: passing 
 an object that undergoes default argument promotion to 'va_start' has 
 undefined behavior [-Wvarargs]
  va_start(Argp,imode);
^
 /home/balay/petsc/src/dm/impls/composite/pack.c:641:61: note: parameter of 
 type 'InsertMode' is declared here
 PetscErrorCode  DMCompositeGather(DM dm,Vec gvec,InsertMode imode,...)
^
 1 warning generated.
 <
>> 
>> The fix here is:
>> 
>> -PetscErrorCode  DMCompositeGather(DM dm,Vec gvec,InsertMode imode,...)
>> +PetscErrorCode  DMCompositeGather(DM dm,InsertMode imode,Vec gvec,...)
>> 
>> [Barry is ok - but Jed,Matt might object?]
> 
> It's annoying, but it appears to be needed.  It's screwy because enum
> ought to be handled no differently than int for this purpose.

   Yeah, we checked and sizeof(enum type) = 4 so there is no need for promotion 
so I blame it on compiler flakiness.



Re: [petsc-dev] Hijacked MPI calls...

2016-09-15 Thread Jed Brown
Satish Balay  writes:

> On Tue, 13 Sep 2016, Satish Balay wrote:
>
>> On Tue, 13 Sep 2016, Satish Balay wrote:
>> 
>> > And this one is perhaps for Matt..
>> 
>> Ops - perhaps Jed needs to look at this..
>> > 
>> > >>>
>> >   CC arch-clang-cmplx/obj/src/dm/impls/composite/pack.o
>> > /home/balay/petsc/src/dm/impls/composite/pack.c:657:17: warning: passing 
>> > an object that undergoes default argument promotion to 'va_start' has 
>> > undefined behavior [-Wvarargs]
>> >   va_start(Argp,imode);
>> > ^
>> > /home/balay/petsc/src/dm/impls/composite/pack.c:641:61: note: parameter of 
>> > type 'InsertMode' is declared here
>> > PetscErrorCode  DMCompositeGather(DM dm,Vec gvec,InsertMode imode,...)
>> > ^
>> > 1 warning generated.
>> >  <
>
> The fix here is:
>
> -PetscErrorCode  DMCompositeGather(DM dm,Vec gvec,InsertMode imode,...)
> +PetscErrorCode  DMCompositeGather(DM dm,InsertMode imode,Vec gvec,...)
>
> [Barry is ok - but Jed,Matt might object?]

It's annoying, but it appears to be needed.  It's screwy because enum
ought to be handled no differently than int for this purpose.


signature.asc
Description: PGP signature


Re: [petsc-dev] Hijacked MPI calls...

2016-09-13 Thread Satish Balay
On Tue, 13 Sep 2016, Satish Balay wrote:

> On Tue, 13 Sep 2016, Satish Balay wrote:
> 
> > And this one is perhaps for Matt..
> 
> Ops - perhaps Jed needs to look at this..
> > 
> > >>>
> >   CC arch-clang-cmplx/obj/src/dm/impls/composite/pack.o
> > /home/balay/petsc/src/dm/impls/composite/pack.c:657:17: warning: passing an 
> > object that undergoes default argument promotion to 'va_start' has 
> > undefined behavior [-Wvarargs]
> >   va_start(Argp,imode);
> > ^
> > /home/balay/petsc/src/dm/impls/composite/pack.c:641:61: note: parameter of 
> > type 'InsertMode' is declared here
> > PetscErrorCode  DMCompositeGather(DM dm,Vec gvec,InsertMode imode,...)
> > ^
> > 1 warning generated.
> >  <

The fix here is:

-PetscErrorCode  DMCompositeGather(DM dm,Vec gvec,InsertMode imode,...)
+PetscErrorCode  DMCompositeGather(DM dm,InsertMode imode,Vec gvec,...)

[Barry is ok - but Jed,Matt might object?]

And I pushed a fix for the -Wexpansion-to-defined issue below..

Satish

> 
> And also:
> 
> 
> balay@asterix /home/balay/petsc/src/snes/examples/tutorials 
> (balay/clang-Wcomma=)
> $ make ex48
> /home/balay/petsc/arch-clang-cmplx/bin/mpicc -o ex48.o -c -Wall 
> -Wwrite-strings -Wno-strict-aliasing -Wno-unknown-pragmas -Qunused-arguments 
> -Wcomma   -g3   -I/home/balay/petsc/include 
> -I/home/balay/petsc/arch-clang-cmplx/include`pwd`/ex48.c
> /home/balay/petsc/src/snes/examples/tutorials/ex48.c:1118:5: warning: macro 
> expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
> #if USE_SSE2_KERNELS
> ^
> /home/balay/petsc/src/snes/examples/tutorials/ex48.c:78:28: note: expanded 
> from macro 'USE_SSE2_KERNELS'
> #define USE_SSE2_KERNELS (!defined NO_SSE2  \
>^
> /home/balay/petsc/src/snes/examples/tutorials/ex48.c:1118:5: warning: macro 
> expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
> /home/balay/petsc/src/snes/examples/tutorials/ex48.c:79:31: note: expanded 
> from macro 'USE_SSE2_KERNELS'
>   && !defined PETSC_USE_COMPLEX \
>   ^
> /home/balay/petsc/src/snes/examples/tutorials/ex48.c:1118:5: warning: macro 
> expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
> /home/balay/petsc/src/snes/examples/tutorials/ex48.c:80:31: note: expanded 
> from macro 'USE_SSE2_KERNELS'
>   && !defined PETSC_USE_REAL_SINGLE \
>   ^
> /home/balay/petsc/src/snes/examples/tutorials/ex48.c:1118:5: warning: macro 
> expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
> /home/balay/petsc/src/snes/examples/tutorials/ex48.c:81:31: note: expanded 
> from macro 'USE_SSE2_KERNELS'
>   && !defined PETSC_USE_REAL___FLOAT128 \
>   ^
> /home/balay/petsc/src/snes/examples/tutorials/ex48.c:1118:5: warning: macro 
> expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
> /home/balay/petsc/src/snes/examples/tutorials/ex48.c:82:30: note: expanded 
> from macro 'USE_SSE2_KERNELS'
>   && defined __SSE2__)
>  ^
> /home/balay/petsc/src/snes/examples/tutorials/ex48.c:1143:6: warning: macro 
> expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
> #if !USE_SSE2_KERNELS
>  ^
> /home/balay/petsc/src/snes/examples/tutorials/ex48.c:78:28: note: expanded 
> from macro 'USE_SSE2_KERNELS'
> #define USE_SSE2_KERNELS (!defined NO_SSE2  \
>^
> /home/balay/petsc/src/snes/examples/tutorials/ex48.c:1143:6: warning: macro 
> expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
> /home/balay/petsc/src/snes/examples/tutorials/ex48.c:79:31: note: expanded 
> from macro 'USE_SSE2_KERNELS'
>   && !defined PETSC_USE_COMPLEX \
>   ^
> /home/balay/petsc/src/snes/examples/tutorials/ex48.c:1143:6: warning: macro 
> expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
> /home/balay/petsc/src/snes/examples/tutorials/ex48.c:80:31: note: expanded 
> from macro 'USE_SSE2_KERNELS'
>   && !defined PETSC_USE_REAL_SINGLE \
>   ^
> /home/balay/petsc/src/snes/examples/tutorials/ex48.c:1143:6: warning: macro 
> expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
> /home/balay/petsc/src/snes/examples/tutorials/ex48.c:81:31: note: expanded 
> from macro 'USE_SSE2_KERNELS'
>   && !defined PETSC_USE_REAL___FLOAT128 \
>   ^
> /home/balay/petsc/src/snes/examples/tutorials/ex48.c:1143:6: warning: macro 
> 

Re: [petsc-dev] Hijacked MPI calls...

2016-09-13 Thread Satish Balay
On Tue, 13 Sep 2016, Satish Balay wrote:

> And this one is perhaps for Matt..

Ops - perhaps Jed needs to look at this..
> 
> >>>
>   CC arch-clang-cmplx/obj/src/dm/impls/composite/pack.o
> /home/balay/petsc/src/dm/impls/composite/pack.c:657:17: warning: passing an 
> object that undergoes default argument promotion to 'va_start' has undefined 
> behavior [-Wvarargs]
>   va_start(Argp,imode);
> ^
> /home/balay/petsc/src/dm/impls/composite/pack.c:641:61: note: parameter of 
> type 'InsertMode' is declared here
> PetscErrorCode  DMCompositeGather(DM dm,Vec gvec,InsertMode imode,...)
> ^
> 1 warning generated.
>  <

And also:


balay@asterix /home/balay/petsc/src/snes/examples/tutorials 
(balay/clang-Wcomma=)
$ make ex48
/home/balay/petsc/arch-clang-cmplx/bin/mpicc -o ex48.o -c -Wall -Wwrite-strings 
-Wno-strict-aliasing -Wno-unknown-pragmas -Qunused-arguments -Wcomma   -g3   
-I/home/balay/petsc/include -I/home/balay/petsc/arch-clang-cmplx/include
`pwd`/ex48.c
/home/balay/petsc/src/snes/examples/tutorials/ex48.c:1118:5: warning: macro 
expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
#if USE_SSE2_KERNELS
^
/home/balay/petsc/src/snes/examples/tutorials/ex48.c:78:28: note: expanded from 
macro 'USE_SSE2_KERNELS'
#define USE_SSE2_KERNELS (!defined NO_SSE2  \
   ^
/home/balay/petsc/src/snes/examples/tutorials/ex48.c:1118:5: warning: macro 
expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
/home/balay/petsc/src/snes/examples/tutorials/ex48.c:79:31: note: expanded from 
macro 'USE_SSE2_KERNELS'
  && !defined PETSC_USE_COMPLEX \
  ^
/home/balay/petsc/src/snes/examples/tutorials/ex48.c:1118:5: warning: macro 
expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
/home/balay/petsc/src/snes/examples/tutorials/ex48.c:80:31: note: expanded from 
macro 'USE_SSE2_KERNELS'
  && !defined PETSC_USE_REAL_SINGLE \
  ^
/home/balay/petsc/src/snes/examples/tutorials/ex48.c:1118:5: warning: macro 
expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
/home/balay/petsc/src/snes/examples/tutorials/ex48.c:81:31: note: expanded from 
macro 'USE_SSE2_KERNELS'
  && !defined PETSC_USE_REAL___FLOAT128 \
  ^
/home/balay/petsc/src/snes/examples/tutorials/ex48.c:1118:5: warning: macro 
expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
/home/balay/petsc/src/snes/examples/tutorials/ex48.c:82:30: note: expanded from 
macro 'USE_SSE2_KERNELS'
  && defined __SSE2__)
 ^
/home/balay/petsc/src/snes/examples/tutorials/ex48.c:1143:6: warning: macro 
expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
#if !USE_SSE2_KERNELS
 ^
/home/balay/petsc/src/snes/examples/tutorials/ex48.c:78:28: note: expanded from 
macro 'USE_SSE2_KERNELS'
#define USE_SSE2_KERNELS (!defined NO_SSE2  \
   ^
/home/balay/petsc/src/snes/examples/tutorials/ex48.c:1143:6: warning: macro 
expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
/home/balay/petsc/src/snes/examples/tutorials/ex48.c:79:31: note: expanded from 
macro 'USE_SSE2_KERNELS'
  && !defined PETSC_USE_COMPLEX \
  ^
/home/balay/petsc/src/snes/examples/tutorials/ex48.c:1143:6: warning: macro 
expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
/home/balay/petsc/src/snes/examples/tutorials/ex48.c:80:31: note: expanded from 
macro 'USE_SSE2_KERNELS'
  && !defined PETSC_USE_REAL_SINGLE \
  ^
/home/balay/petsc/src/snes/examples/tutorials/ex48.c:1143:6: warning: macro 
expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
/home/balay/petsc/src/snes/examples/tutorials/ex48.c:81:31: note: expanded from 
macro 'USE_SSE2_KERNELS'
  && !defined PETSC_USE_REAL___FLOAT128 \
  ^
/home/balay/petsc/src/snes/examples/tutorials/ex48.c:1143:6: warning: macro 
expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
/home/balay/petsc/src/snes/examples/tutorials/ex48.c:82:30: note: expanded from 
macro 'USE_SSE2_KERNELS'
  && defined __SSE2__)
 ^
10 warnings generated.
/home/balay/petsc/arch-clang-cmplx/bin/mpicc -Wall -Wwrite-strings 
-Wno-strict-aliasing -Wno-unknown-pragmas -Qunused-arguments -Wcomma   -g3  -o 
ex48 ex48.o -Wl,-rpath,/home/balay/petsc/arch-clang-cmplx/lib 
-L/home

Re: [petsc-dev] Hijacked MPI calls...

2016-09-13 Thread Satish Balay
Ok I used your notation - and (void) only for the stuff inside while/for loops.

https://bitbucket.org/petsc/petsc/pull-requests/561/balay-clang-wcomma/diff


Satish

On Tue, 13 Sep 2016, Barry Smith wrote:

> 
>I just had a bad experience with (void) so feel it is only a half-answer.
> 
> 
> > On Sep 13, 2016, at 10:45 AM, Satish Balay  wrote:
> > 
> > On Tue, 13 Sep 2016, Barry Smith wrote:
> > 
> >> 
> >>> On Sep 13, 2016, at 10:13 AM, Satish Balay  wrote:
> >>> 
> >>> There is stuff like:
> >>> 
> >>> src/mat/order/fn1wd.c
> >>> 
> >>> -kstop = (i__2 = xadj[node + 1], (PetscInt)PetscAbsInt(i__2)) - 1;
> >>> +kstop = (PetscInt)PetscAbsInt(xadj[node + 1]) - 1;
> >>> 
> >>> Perhaps PetscAbsInt() type stuff should be switched over to static inline?
> >>> Or use a different notation?
> >>> 
> >>> i__2  = xadj[node + 1];
> >>> kstop = (PetscInt)PetscAbsInt(i__2) - 1;
> >> 
> >>   This is the way any normal person would write it :-)
> > 
> > I think the first one is what is more intutive. The second one is
> > clearly encoding an optimization - as the code is aware PetscAbsInt()
> > is a macro - not a proper function. Hence the thought about 'static inline'
> > 
> > The first change compiles fine [with clang - both c, complex - but
> > don't know if there are problem compilers]
> > 
> > I'm now inclined to just preserve the current usage with void:
> > 
> >kstop = ((void)(i__2 = xadj[node + 1]), (PetscInt)PetscAbsInt(i__2)) - 1;
> > 
> >> 
> >>> 
> >>> And for the following - use 'void' as compiler suggests?
> >>> 
> >>> a/src/mat/utils/pheap.c
> >>> -  while (par = Parent(loc), Value(h,par) > val) {
> >>> +  while ((void)(par = Parent(loc)), Value(h,par) > val) {
> >>> 
> >>> a/src/dm/impls/plex/plexdistribute.c
> >>> -  for (q = 0; q < numAdj || (adj[numAdj++] = support[s],0); ++q) {
> >>> +  for (q = 0; q < numAdj || ((void)(adj[numAdj++] = support[s]),0); 
> >>> ++q) {
> >> 
> >>   Does the (void) work to eliminate the warning?
> > 
> > yes.
> > 
> > Satish
> > 
> >> 
> >>  Barry
> >> 
> >>> 
> >>> 
> >>> And this one is perhaps for Matt..
> >>> 
> >> 
> >>> CC arch-clang-cmplx/obj/src/dm/impls/composite/pack.o
> >>> /home/balay/petsc/src/dm/impls/composite/pack.c:657:17: warning: passing 
> >>> an object that undergoes default argument promotion to 'va_start' has 
> >>> undefined behavior [-Wvarargs]
> >>> va_start(Argp,imode);
> >>>   ^
> >>> /home/balay/petsc/src/dm/impls/composite/pack.c:641:61: note: parameter 
> >>> of type 'InsertMode' is declared here
> >>> PetscErrorCode  DMCompositeGather(DM dm,Vec gvec,InsertMode imode,...)
> >>>   ^
> >>> 1 warning generated.
> >>> <
> >>> 
> >>> 
> >>> 
> >>> Satish
> >>> 
> >>> On Mon, 12 Sep 2016, Barry Smith wrote:
> >>> 
>  
>  Satish,
>  
>   Interesting. We can probably get rid of most of the other warnings 
>  easily; it looks like mostly frivolous use of the , notation.
>  
>   Barry
>  
> > On Sep 12, 2016, at 11:19 PM, Satish Balay  wrote:
> > 
> > The attached patch gets rid of most of the warnings [esp MPI usage
> > from PETSc code - with the logging wrappers].
> > 
> > [also added to balay/clang-Wcomma]
> > 
> > Satish
> > 
> > On Mon, 12 Sep 2016, Satish Balay wrote:
> > 
> >> ok - so you are using CFLAGS=-Wcomma on your build..
> >> 
> >> I see warnings now. Attaching make.log
> >> 
> >> Satish
> >> 
> >> On Mon, 12 Sep 2016, Eric Chamberland wrote:
> >> 
> >>> 
> >>> 
> >>> Le 2016-09-12 à 17:32, Satish Balay a écrit :
>  Do you get these warnings with PETSc library build aswell?
> >>> I can't tell since I didn't tried to build PETSc with clang...
>  
>  The logging code tries to log all messages in library and in
>  application - and prints a summary with -info.
>  
>  You can disable logging in your build with configure option: 
>  --with-log=0
>  
>  Or you can define the PETSC_HAVE_BROKEN_RECURSIVE_MACRO in your
>  code/compile - and the wrappers will be skipped..
> >>> ok, so these may be good workaround for me to keep -Wcomma activated 
> >>> on our
> >>> "push server" that filters incoming commits...
> >>> 
> >>> Thanks!!!
> >>> 
> >>> Eric
> >>> 
> >>> 
> >>> 
> >> 
> > 
> 
> 


Re: [petsc-dev] Hijacked MPI calls...

2016-09-13 Thread Barry Smith

   I just had a bad experience with (void) so feel it is only a half-answer.


> On Sep 13, 2016, at 10:45 AM, Satish Balay  wrote:
> 
> On Tue, 13 Sep 2016, Barry Smith wrote:
> 
>> 
>>> On Sep 13, 2016, at 10:13 AM, Satish Balay  wrote:
>>> 
>>> There is stuff like:
>>> 
>>> src/mat/order/fn1wd.c
>>> 
>>> -kstop = (i__2 = xadj[node + 1], (PetscInt)PetscAbsInt(i__2)) - 1;
>>> +kstop = (PetscInt)PetscAbsInt(xadj[node + 1]) - 1;
>>> 
>>> Perhaps PetscAbsInt() type stuff should be switched over to static inline?
>>> Or use a different notation?
>>> 
>>> i__2  = xadj[node + 1];
>>> kstop = (PetscInt)PetscAbsInt(i__2) - 1;
>> 
>>   This is the way any normal person would write it :-)
> 
> I think the first one is what is more intutive. The second one is
> clearly encoding an optimization - as the code is aware PetscAbsInt()
> is a macro - not a proper function. Hence the thought about 'static inline'
> 
> The first change compiles fine [with clang - both c, complex - but
> don't know if there are problem compilers]
> 
> I'm now inclined to just preserve the current usage with void:
> 
>kstop = ((void)(i__2 = xadj[node + 1]), (PetscInt)PetscAbsInt(i__2)) - 1;
> 
>> 
>>> 
>>> And for the following - use 'void' as compiler suggests?
>>> 
>>> a/src/mat/utils/pheap.c
>>> -  while (par = Parent(loc), Value(h,par) > val) {
>>> +  while ((void)(par = Parent(loc)), Value(h,par) > val) {
>>> 
>>> a/src/dm/impls/plex/plexdistribute.c
>>> -  for (q = 0; q < numAdj || (adj[numAdj++] = support[s],0); ++q) {
>>> +  for (q = 0; q < numAdj || ((void)(adj[numAdj++] = support[s]),0); 
>>> ++q) {
>> 
>>   Does the (void) work to eliminate the warning?
> 
> yes.
> 
> Satish
> 
>> 
>>  Barry
>> 
>>> 
>>> 
>>> And this one is perhaps for Matt..
>>> 
>> 
>>> CC arch-clang-cmplx/obj/src/dm/impls/composite/pack.o
>>> /home/balay/petsc/src/dm/impls/composite/pack.c:657:17: warning: passing an 
>>> object that undergoes default argument promotion to 'va_start' has 
>>> undefined behavior [-Wvarargs]
>>> va_start(Argp,imode);
>>>   ^
>>> /home/balay/petsc/src/dm/impls/composite/pack.c:641:61: note: parameter of 
>>> type 'InsertMode' is declared here
>>> PetscErrorCode  DMCompositeGather(DM dm,Vec gvec,InsertMode imode,...)
>>>   ^
>>> 1 warning generated.
>>> <
>>> 
>>> 
>>> 
>>> Satish
>>> 
>>> On Mon, 12 Sep 2016, Barry Smith wrote:
>>> 
 
 Satish,
 
  Interesting. We can probably get rid of most of the other warnings 
 easily; it looks like mostly frivolous use of the , notation.
 
  Barry
 
> On Sep 12, 2016, at 11:19 PM, Satish Balay  wrote:
> 
> The attached patch gets rid of most of the warnings [esp MPI usage
> from PETSc code - with the logging wrappers].
> 
> [also added to balay/clang-Wcomma]
> 
> Satish
> 
> On Mon, 12 Sep 2016, Satish Balay wrote:
> 
>> ok - so you are using CFLAGS=-Wcomma on your build..
>> 
>> I see warnings now. Attaching make.log
>> 
>> Satish
>> 
>> On Mon, 12 Sep 2016, Eric Chamberland wrote:
>> 
>>> 
>>> 
>>> Le 2016-09-12 à 17:32, Satish Balay a écrit :
 Do you get these warnings with PETSc library build aswell?
>>> I can't tell since I didn't tried to build PETSc with clang...
 
 The logging code tries to log all messages in library and in
 application - and prints a summary with -info.
 
 You can disable logging in your build with configure option: 
 --with-log=0
 
 Or you can define the PETSC_HAVE_BROKEN_RECURSIVE_MACRO in your
 code/compile - and the wrappers will be skipped..
>>> ok, so these may be good workaround for me to keep -Wcomma activated on 
>>> our
>>> "push server" that filters incoming commits...
>>> 
>>> Thanks!!!
>>> 
>>> Eric
>>> 
>>> 
>>> 
>> 
> 



Re: [petsc-dev] Hijacked MPI calls...

2016-09-13 Thread Satish Balay
On Tue, 13 Sep 2016, Barry Smith wrote:

> 
> > On Sep 13, 2016, at 10:13 AM, Satish Balay  wrote:
> > 
> > There is stuff like:
> > 
> > src/mat/order/fn1wd.c
> > 
> > -kstop = (i__2 = xadj[node + 1], (PetscInt)PetscAbsInt(i__2)) - 1;
> > +kstop = (PetscInt)PetscAbsInt(xadj[node + 1]) - 1;
> > 
> > Perhaps PetscAbsInt() type stuff should be switched over to static inline?
> > Or use a different notation?
> > 
> > i__2  = xadj[node + 1];
> > kstop = (PetscInt)PetscAbsInt(i__2) - 1;
> 
>This is the way any normal person would write it :-)

I think the first one is what is more intutive. The second one is
clearly encoding an optimization - as the code is aware PetscAbsInt()
is a macro - not a proper function. Hence the thought about 'static inline'

The first change compiles fine [with clang - both c, complex - but
don't know if there are problem compilers]

I'm now inclined to just preserve the current usage with void:

kstop = ((void)(i__2 = xadj[node + 1]), (PetscInt)PetscAbsInt(i__2)) - 1;

> 
> > 
> > And for the following - use 'void' as compiler suggests?
> > 
> > a/src/mat/utils/pheap.c
> > -  while (par = Parent(loc), Value(h,par) > val) {
> > +  while ((void)(par = Parent(loc)), Value(h,par) > val) {
> > 
> > a/src/dm/impls/plex/plexdistribute.c
> > -  for (q = 0; q < numAdj || (adj[numAdj++] = support[s],0); ++q) {
> > +  for (q = 0; q < numAdj || ((void)(adj[numAdj++] = support[s]),0); 
> > ++q) {
> 
>Does the (void) work to eliminate the warning?

yes.

Satish

> 
>   Barry
> 
> > 
> > 
> > And this one is perhaps for Matt..
> > 
>  
> >  CC arch-clang-cmplx/obj/src/dm/impls/composite/pack.o
> > /home/balay/petsc/src/dm/impls/composite/pack.c:657:17: warning: passing an 
> > object that undergoes default argument promotion to 'va_start' has 
> > undefined behavior [-Wvarargs]
> >  va_start(Argp,imode);
> >^
> > /home/balay/petsc/src/dm/impls/composite/pack.c:641:61: note: parameter of 
> > type 'InsertMode' is declared here
> > PetscErrorCode  DMCompositeGather(DM dm,Vec gvec,InsertMode imode,...)
> >^
> > 1 warning generated.
> > <
> > 
> > 
> > 
> > Satish
> > 
> > On Mon, 12 Sep 2016, Barry Smith wrote:
> > 
> >> 
> >>  Satish,
> >> 
> >>   Interesting. We can probably get rid of most of the other warnings 
> >> easily; it looks like mostly frivolous use of the , notation.
> >> 
> >>   Barry
> >> 
> >>> On Sep 12, 2016, at 11:19 PM, Satish Balay  wrote:
> >>> 
> >>> The attached patch gets rid of most of the warnings [esp MPI usage
> >>> from PETSc code - with the logging wrappers].
> >>> 
> >>> [also added to balay/clang-Wcomma]
> >>> 
> >>> Satish
> >>> 
> >>> On Mon, 12 Sep 2016, Satish Balay wrote:
> >>> 
>  ok - so you are using CFLAGS=-Wcomma on your build..
>  
>  I see warnings now. Attaching make.log
>  
>  Satish
>  
>  On Mon, 12 Sep 2016, Eric Chamberland wrote:
>  
> > 
> > 
> > Le 2016-09-12 à 17:32, Satish Balay a écrit :
> >> Do you get these warnings with PETSc library build aswell?
> > I can't tell since I didn't tried to build PETSc with clang...
> >> 
> >> The logging code tries to log all messages in library and in
> >> application - and prints a summary with -info.
> >> 
> >> You can disable logging in your build with configure option: 
> >> --with-log=0
> >> 
> >> Or you can define the PETSC_HAVE_BROKEN_RECURSIVE_MACRO in your
> >> code/compile - and the wrappers will be skipped..
> > ok, so these may be good workaround for me to keep -Wcomma activated on 
> > our
> > "push server" that filters incoming commits...
> > 
> > Thanks!!!
> > 
> > Eric
> > 
> > 
> > 
>  
> >>> 
> >> 
> 
> 


Re: [petsc-dev] Hijacked MPI calls...

2016-09-13 Thread Barry Smith

> On Sep 13, 2016, at 10:13 AM, Satish Balay  wrote:
> 
> There is stuff like:
> 
> src/mat/order/fn1wd.c
> 
> -kstop = (i__2 = xadj[node + 1], (PetscInt)PetscAbsInt(i__2)) - 1;
> +kstop = (PetscInt)PetscAbsInt(xadj[node + 1]) - 1;
> 
> Perhaps PetscAbsInt() type stuff should be switched over to static inline?
> Or use a different notation?
> 
> i__2  = xadj[node + 1];
> kstop = (PetscInt)PetscAbsInt(i__2) - 1;

   This is the way any normal person would write it :-)

> 
> And for the following - use 'void' as compiler suggests?
> 
> a/src/mat/utils/pheap.c
> -  while (par = Parent(loc), Value(h,par) > val) {
> +  while ((void)(par = Parent(loc)), Value(h,par) > val) {
> 
> a/src/dm/impls/plex/plexdistribute.c
> -  for (q = 0; q < numAdj || (adj[numAdj++] = support[s],0); ++q) {
> +  for (q = 0; q < numAdj || ((void)(adj[numAdj++] = support[s]),0); ++q) 
> {

   Does the (void) work to eliminate the warning?

  Barry

> 
> 
> And this one is perhaps for Matt..
> 
 
>  CC arch-clang-cmplx/obj/src/dm/impls/composite/pack.o
> /home/balay/petsc/src/dm/impls/composite/pack.c:657:17: warning: passing an 
> object that undergoes default argument promotion to 'va_start' has undefined 
> behavior [-Wvarargs]
>  va_start(Argp,imode);
>^
> /home/balay/petsc/src/dm/impls/composite/pack.c:641:61: note: parameter of 
> type 'InsertMode' is declared here
> PetscErrorCode  DMCompositeGather(DM dm,Vec gvec,InsertMode imode,...)
>^
> 1 warning generated.
> <
> 
> 
> 
> Satish
> 
> On Mon, 12 Sep 2016, Barry Smith wrote:
> 
>> 
>>  Satish,
>> 
>>   Interesting. We can probably get rid of most of the other warnings easily; 
>> it looks like mostly frivolous use of the , notation.
>> 
>>   Barry
>> 
>>> On Sep 12, 2016, at 11:19 PM, Satish Balay  wrote:
>>> 
>>> The attached patch gets rid of most of the warnings [esp MPI usage
>>> from PETSc code - with the logging wrappers].
>>> 
>>> [also added to balay/clang-Wcomma]
>>> 
>>> Satish
>>> 
>>> On Mon, 12 Sep 2016, Satish Balay wrote:
>>> 
 ok - so you are using CFLAGS=-Wcomma on your build..
 
 I see warnings now. Attaching make.log
 
 Satish
 
 On Mon, 12 Sep 2016, Eric Chamberland wrote:
 
> 
> 
> Le 2016-09-12 à 17:32, Satish Balay a écrit :
>> Do you get these warnings with PETSc library build aswell?
> I can't tell since I didn't tried to build PETSc with clang...
>> 
>> The logging code tries to log all messages in library and in
>> application - and prints a summary with -info.
>> 
>> You can disable logging in your build with configure option: --with-log=0
>> 
>> Or you can define the PETSC_HAVE_BROKEN_RECURSIVE_MACRO in your
>> code/compile - and the wrappers will be skipped..
> ok, so these may be good workaround for me to keep -Wcomma activated on 
> our
> "push server" that filters incoming commits...
> 
> Thanks!!!
> 
> Eric
> 
> 
> 
 
>>> 
>> 



Re: [petsc-dev] Hijacked MPI calls...

2016-09-13 Thread Satish Balay
There is stuff like:

src/mat/order/fn1wd.c

-kstop = (i__2 = xadj[node + 1], (PetscInt)PetscAbsInt(i__2)) - 1;
+kstop = (PetscInt)PetscAbsInt(xadj[node + 1]) - 1;

Perhaps PetscAbsInt() type stuff should be switched over to static inline?
Or use a different notation?

i__2  = xadj[node + 1];
kstop = (PetscInt)PetscAbsInt(i__2) - 1;

And for the following - use 'void' as compiler suggests?

a/src/mat/utils/pheap.c
-  while (par = Parent(loc), Value(h,par) > val) {
+  while ((void)(par = Parent(loc)), Value(h,par) > val) {

a/src/dm/impls/plex/plexdistribute.c
-  for (q = 0; q < numAdj || (adj[numAdj++] = support[s],0); ++q) {
+  for (q = 0; q < numAdj || ((void)(adj[numAdj++] = support[s]),0); ++q) {


And this one is perhaps for Matt..

>>>
  CC arch-clang-cmplx/obj/src/dm/impls/composite/pack.o
/home/balay/petsc/src/dm/impls/composite/pack.c:657:17: warning: passing an 
object that undergoes default argument promotion to 'va_start' has undefined 
behavior [-Wvarargs]
  va_start(Argp,imode);
^
/home/balay/petsc/src/dm/impls/composite/pack.c:641:61: note: parameter of type 
'InsertMode' is declared here
PetscErrorCode  DMCompositeGather(DM dm,Vec gvec,InsertMode imode,...)
^
1 warning generated.
 <



Satish

On Mon, 12 Sep 2016, Barry Smith wrote:

> 
>   Satish,
> 
>Interesting. We can probably get rid of most of the other warnings easily; 
> it looks like mostly frivolous use of the , notation.
> 
>Barry
> 
> > On Sep 12, 2016, at 11:19 PM, Satish Balay  wrote:
> > 
> > The attached patch gets rid of most of the warnings [esp MPI usage
> > from PETSc code - with the logging wrappers].
> > 
> > [also added to balay/clang-Wcomma]
> > 
> > Satish
> > 
> > On Mon, 12 Sep 2016, Satish Balay wrote:
> > 
> >> ok - so you are using CFLAGS=-Wcomma on your build..
> >> 
> >> I see warnings now. Attaching make.log
> >> 
> >> Satish
> >> 
> >> On Mon, 12 Sep 2016, Eric Chamberland wrote:
> >> 
> >>> 
> >>> 
> >>> Le 2016-09-12 à 17:32, Satish Balay a écrit :
>  Do you get these warnings with PETSc library build aswell?
> >>> I can't tell since I didn't tried to build PETSc with clang...
>  
>  The logging code tries to log all messages in library and in
>  application - and prints a summary with -info.
>  
>  You can disable logging in your build with configure option: --with-log=0
>  
>  Or you can define the PETSC_HAVE_BROKEN_RECURSIVE_MACRO in your
>  code/compile - and the wrappers will be skipped..
> >>> ok, so these may be good workaround for me to keep -Wcomma activated on 
> >>> our
> >>> "push server" that filters incoming commits...
> >>> 
> >>> Thanks!!!
> >>> 
> >>> Eric
> >>> 
> >>> 
> >>> 
> >> 
> > 
> 
> 

Re: [petsc-dev] Hijacked MPI calls...

2016-09-12 Thread Barry Smith

  Satish,

   Interesting. We can probably get rid of most of the other warnings easily; 
it looks like mostly frivolous use of the , notation.

   Barry

> On Sep 12, 2016, at 11:19 PM, Satish Balay  wrote:
> 
> The attached patch gets rid of most of the warnings [esp MPI usage
> from PETSc code - with the logging wrappers].
> 
> [also added to balay/clang-Wcomma]
> 
> Satish
> 
> On Mon, 12 Sep 2016, Satish Balay wrote:
> 
>> ok - so you are using CFLAGS=-Wcomma on your build..
>> 
>> I see warnings now. Attaching make.log
>> 
>> Satish
>> 
>> On Mon, 12 Sep 2016, Eric Chamberland wrote:
>> 
>>> 
>>> 
>>> Le 2016-09-12 à 17:32, Satish Balay a écrit :
 Do you get these warnings with PETSc library build aswell?
>>> I can't tell since I didn't tried to build PETSc with clang...
 
 The logging code tries to log all messages in library and in
 application - and prints a summary with -info.
 
 You can disable logging in your build with configure option: --with-log=0
 
 Or you can define the PETSC_HAVE_BROKEN_RECURSIVE_MACRO in your
 code/compile - and the wrappers will be skipped..
>>> ok, so these may be good workaround for me to keep -Wcomma activated on our
>>> "push server" that filters incoming commits...
>>> 
>>> Thanks!!!
>>> 
>>> Eric
>>> 
>>> 
>>> 
>> 
> 



Re: [petsc-dev] Hijacked MPI calls...

2016-09-12 Thread Satish Balay
The attached patch gets rid of most of the warnings [esp MPI usage
from PETSc code - with the logging wrappers].

[also added to balay/clang-Wcomma]

Satish

On Mon, 12 Sep 2016, Satish Balay wrote:

> ok - so you are using CFLAGS=-Wcomma on your build..
> 
> I see warnings now. Attaching make.log
> 
> Satish
> 
> On Mon, 12 Sep 2016, Eric Chamberland wrote:
> 
> > 
> > 
> > Le 2016-09-12 à 17:32, Satish Balay a écrit :
> > > Do you get these warnings with PETSc library build aswell?
> > I can't tell since I didn't tried to build PETSc with clang...
> > >
> > > The logging code tries to log all messages in library and in
> > > application - and prints a summary with -info.
> > >
> > > You can disable logging in your build with configure option: --with-log=0
> > >
> > > Or you can define the PETSC_HAVE_BROKEN_RECURSIVE_MACRO in your
> > > code/compile - and the wrappers will be skipped..
> > ok, so these may be good workaround for me to keep -Wcomma activated on our
> > "push server" that filters incoming commits...
> > 
> > Thanks!!!
> > 
> > Eric
> > 
> > 
> > 
> 
diff --git a/include/petscbt.h b/include/petscbt.h
index 6d08fd5..9904f03 100644
--- a/include/petscbt.h
+++ b/include/petscbt.h
@@ -49,10 +49,10 @@ PETSC_STATIC_INLINE char PetscBTLookup(PetscBT 
array,PetscInt index)
   char  BT_mask,BT_c;
   PetscInt  BT_idx;
 
- return  (BT_idx= index/PETSC_BITS_PER_BYTE,
-  BT_c  = array[BT_idx],
-  BT_mask   = (char)(1 << index%PETSC_BITS_PER_BYTE),
-  (char)(BT_c & BT_mask));
+  BT_idx= index/PETSC_BITS_PER_BYTE;
+  BT_c  = array[BT_idx];
+  BT_mask   = (char)(1 << index%PETSC_BITS_PER_BYTE);
+  return (char)(BT_c & BT_mask);
 }
 
 PETSC_STATIC_INLINE PetscErrorCode PetscBTView(PetscInt m,const PetscBT 
bt,PetscViewer viewer)
@@ -80,11 +80,11 @@ PETSC_STATIC_INLINE char PetscBTLookupSet(PetscBT 
array,PetscInt index)
   char  BT_mask,BT_c;
   PetscInt  BT_idx;
 
-  return (BT_idx= index/PETSC_BITS_PER_BYTE,
-  BT_c  = array[BT_idx],
-  BT_mask   = (char)(1 << index%PETSC_BITS_PER_BYTE),
-  array[BT_idx] = (char)(BT_c | BT_mask),
-  (char)(BT_c & BT_mask));
+  BT_idx= index/PETSC_BITS_PER_BYTE;
+  BT_c  = array[BT_idx];
+  BT_mask   = (char)(1 << index%PETSC_BITS_PER_BYTE);
+  array[BT_idx] = (char)(BT_c | BT_mask);
+  return(char)(BT_c & BT_mask);
 }
 
 PETSC_STATIC_INLINE PetscErrorCode PetscBTSet(PetscBT array,PetscInt index)
@@ -113,11 +113,11 @@ PETSC_STATIC_INLINE char PetscBTLookupClear(PetscBT 
array,PetscInt index)
   char  BT_mask,BT_c;
   PetscInt  BT_idx;
 
-  return (BT_idx= index/PETSC_BITS_PER_BYTE,
-  BT_c  = array[BT_idx],
-  BT_mask   = (char)(1 << index%PETSC_BITS_PER_BYTE),
-  array[BT_idx] = (char)(BT_c & ~BT_mask),
-  (char)(BT_c & BT_mask));
+  BT_idx= index/PETSC_BITS_PER_BYTE;
+  BT_c  = array[BT_idx];
+  BT_mask   = (char)(1 << index%PETSC_BITS_PER_BYTE);
+  array[BT_idx] = (char)(BT_c & ~BT_mask);
+  return (char)(BT_c & BT_mask);
 }
 
 PETSC_STATIC_INLINE PetscErrorCode PetscBTClear(PetscBT array,PetscInt index)
diff --git a/include/petsclog.h b/include/petsclog.h
index 76c7332..2a8658d 100644
--- a/include/petsclog.h
+++ b/include/petsclog.h
@@ -314,9 +314,12 @@ PETSC_EXTERN PetscErrorCode 
PetscLogEventZeroFlops(PetscLogEvent);
 */
 PETSC_STATIC_INLINE PetscErrorCode PetscMPITypeSize(PetscLogDouble 
*buff,PetscMPIInt count,MPI_Datatype type)
 {
-  PetscMPIInt mysize; 
+  PetscMPIInt mysize;
+  PetscErrorCode _myierr;
   if (type == MPI_DATATYPE_NULL) return 0;
-  else return  (MPI_Type_size(type,&mysize) || ((*buff += (PetscLogDouble) 
(count*mysize)),0));
+  _myierr = MPI_Type_size(type,&mysize);CHKERRQ(_myierr);
+  *buff += (PetscLogDouble) (count*mysize);
+  return 0;
 }
 
 PETSC_STATIC_INLINE PetscErrorCode PetscMPITypeSizeComm(MPI_Comm comm, 
PetscLogDouble *buff,PetscMPIInt *counts,MPI_Datatype type)
gmake[1]: Entering directory '/home/balay/petsc'
==
 
See documentation/faq.html and documentation/bugreporting.html
for help with installation problems.  Please send EVERYTHING
printed out below when reporting problems.  Please check the
mailing list archives and consider subscribing.
 
  http://www.mcs.anl.gov/petsc/miscellaneous/mailing-lists.html
 
==
Starting on asterix at Mon Sep 12 23:14:47 CDT 2016
Machine characteristics: Linux asterix 4.8.0-0.rc5.git4.1.fc25.x86_64 #1 SMP 
Fri Sep 9 22:08:28 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
-
Using PETSc directory: /home/balay/petsc
Using PETSc arch: arch-clang
-
PETSC_VERSION_RELEASE0
PETSC_VERSION_MAJOR  3
PETSC_VERSION_MINOR  7
PETSC_VERSION_SUBMINOR   3
PETSC_VERSION_PATCH  0
PETSC_VERSION_DATE   "unknown"
PETSC_VERSION_

Re: [petsc-dev] Hijacked MPI calls...

2016-09-12 Thread Eric Chamberland



Le 2016-09-12 à 22:29, Barry Smith a écrit :

   It's funny we've been doing this for almost 20 years and no one has noticed 
before

but we are only 11 days after clang 3.9.0 release... :)

Eric



Re: [petsc-dev] Hijacked MPI calls...

2016-09-12 Thread Eric Chamberland



Le 2016-09-12 à 17:32, Satish Balay a écrit :

Do you get these warnings with PETSc library build aswell?

I can't tell since I didn't tried to build PETSc with clang...


The logging code tries to log all messages in library and in
application - and prints a summary with -info.

You can disable logging in your build with configure option: --with-log=0

Or you can define the PETSC_HAVE_BROKEN_RECURSIVE_MACRO in your
code/compile - and the wrappers will be skipped..
ok, so these may be good workaround for me to keep -Wcomma activated on 
our "push server" that filters incoming commits...


Thanks!!!

Eric



Re: [petsc-dev] Hijacked MPI calls...

2016-09-12 Thread Satish Balay
I'd still like to see make.log to see if the warnings are also from
petsc build aswell.

[if its clean - as I can't reporduce it with my build with clang-3.9]
I suspect - if MPI calls follow the same notation as petsc sources -
the warnings would go away.

ierr = MPI_Call();CHKERRQ(ierr)

Satish

On Mon, 12 Sep 2016, Barry Smith wrote:

> 
>   We might be able to move it into something like petsclogimpl.h without too 
> much pain. The key is to make it easy to be included in all the PETSc source 
> code but not user code without requiring any convoluted code.
> 
>   It's funny we've been doing this for almost 20 years and no one has noticed 
> before :-)
> 
> 
>   Barry
> 
> > On Sep 12, 2016, at 9:22 PM, Jed Brown  wrote:
> > 
> > Eric Chamberland  writes:
> >> - Would you consider doing static_cast( ) as suggested by clang to 
> >> silence the warnings?
> > 
> > We want valid C so it'll be ((void)petsc_send_ct++,0).  It's annoying,
> > but I think we should do it.  There will be a ton of places inside
> > PETSc.
> > 
> >> - Does PETSc lib really wants to count *my* calls to MPI functions?
> > 
> > It's useful information about what happens inside SNESFunctionEval, for
> > example.  I don't like that it's messing with something that is not in
> > PETSc's namespace and would collide with any other library trying to do
> > the same thing.  So by that metric, it's wrong and we should move it to
> > a private header.
> 
> 



Re: [petsc-dev] Hijacked MPI calls...

2016-09-12 Thread Barry Smith

  We might be able to move it into something like petsclogimpl.h without too 
much pain. The key is to make it easy to be included in all the PETSc source 
code but not user code without requiring any convoluted code.

  It's funny we've been doing this for almost 20 years and no one has noticed 
before :-)


  Barry

> On Sep 12, 2016, at 9:22 PM, Jed Brown  wrote:
> 
> Eric Chamberland  writes:
>> - Would you consider doing static_cast( ) as suggested by clang to 
>> silence the warnings?
> 
> We want valid C so it'll be ((void)petsc_send_ct++,0).  It's annoying,
> but I think we should do it.  There will be a ton of places inside
> PETSc.
> 
>> - Does PETSc lib really wants to count *my* calls to MPI functions?
> 
> It's useful information about what happens inside SNESFunctionEval, for
> example.  I don't like that it's messing with something that is not in
> PETSc's namespace and would collide with any other library trying to do
> the same thing.  So by that metric, it's wrong and we should move it to
> a private header.



Re: [petsc-dev] Hijacked MPI calls...

2016-09-12 Thread Jed Brown
Eric Chamberland  writes:
> - Would you consider doing static_cast( ) as suggested by clang to 
> silence the warnings?

We want valid C so it'll be ((void)petsc_send_ct++,0).  It's annoying,
but I think we should do it.  There will be a ton of places inside
PETSc.

> - Does PETSc lib really wants to count *my* calls to MPI functions?

It's useful information about what happens inside SNESFunctionEval, for
example.  I don't like that it's messing with something that is not in
PETSc's namespace and would collide with any other library trying to do
the same thing.  So by that metric, it's wrong and we should move it to
a private header.


signature.asc
Description: PGP signature


Re: [petsc-dev] Hijacked MPI calls...

2016-09-12 Thread Satish Balay
Do you get these warnings with PETSc library build aswell?

The logging code tries to log all messages in library and in
application - and prints a summary with -info.

You can disable logging in your build with configure option: --with-log=0

Or you can define the PETSC_HAVE_BROKEN_RECURSIVE_MACRO in your
code/compile - and the wrappers will be skipped..

Satish

On Mon, 12 Sep 2016, Eric Chamberland wrote:

> Hi,
> 
> I just discovered that all my calls to MPI are replaced by PETSc #define
> macros...
> 
> I don't really care, but I now have this warning with clang 3.9.0:
> 
> 
> /pmi/cmpbib/compilation_BIB_gcc_redhat_squash/COMPILE_AUTO/GIREF/src/commun/Parallele/PABroadcast.icc:324:41:
> warning: possible misuse of comma operator here [-Wcomma]
>int lErreurMPI = ((petsc_wait_ct++,petsc_sum_of_waits_ct++,0) ||
> MPI_Wait(&(aIdComm[0]),&lStatus));
> ^
> 
> /pmi/cmpbib/compilation_BIB_gcc_redhat_squash/COMPILE_AUTO/GIREF/src/commun/Parallele/PABroadcast.icc:324:26:
> note: cast expression to void to silence warning
>int lErreurMPI = ((petsc_wait_ct++,petsc_sum_of_waits_ct++,0) ||
> MPI_Wait(&(aIdComm[0]),&lStatus));
>  ^~~
>  static_cast( )
> 
> I have many files to modify in my code if I want to silence this warning (with
> pragmas)...
> 
> But before doing so, I have 2 questions:
> 
> - Would you consider doing static_cast( ) as suggested by clang to
> silence the warnings?
> - Does PETSc lib really wants to count *my* calls to MPI functions?
> 
> I don't have a strong opinion if PETSc should #define or not all MPI calls,
> but about -Wcomma warnings, I do: I want this warning to be signaled into my
> code because all the ones I have found where unwanted uses of coma operator...
> and I want to keep the code clean into the future...
> 
> The "offending" macros are all in petsclog.h which is included by petscsys.h.
> 
> thanks,
> 
> Eric
> 
> 



[petsc-dev] Hijacked MPI calls...

2016-09-12 Thread Eric Chamberland

Hi,

I just discovered that all my calls to MPI are replaced by PETSc #define 
macros...


I don't really care, but I now have this warning with clang 3.9.0:


/pmi/cmpbib/compilation_BIB_gcc_redhat_squash/COMPILE_AUTO/GIREF/src/commun/Parallele/PABroadcast.icc:324:41: 
warning: possible misuse of comma operator here [-Wcomma]
   int lErreurMPI = ((petsc_wait_ct++,petsc_sum_of_waits_ct++,0) || 
MPI_Wait(&(aIdComm[0]),&lStatus));

^

/pmi/cmpbib/compilation_BIB_gcc_redhat_squash/COMPILE_AUTO/GIREF/src/commun/Parallele/PABroadcast.icc:324:26: 
note: cast expression to void to silence warning
   int lErreurMPI = ((petsc_wait_ct++,petsc_sum_of_waits_ct++,0) || 
MPI_Wait(&(aIdComm[0]),&lStatus));

 ^~~
 static_cast( )

I have many files to modify in my code if I want to silence this warning 
(with pragmas)...


But before doing so, I have 2 questions:

- Would you consider doing static_cast( ) as suggested by clang to 
silence the warnings?

- Does PETSc lib really wants to count *my* calls to MPI functions?

I don't have a strong opinion if PETSc should #define or not all MPI 
calls, but about -Wcomma warnings, I do: I want this warning to be 
signaled into my code because all the ones I have found where unwanted 
uses of coma operator... and I want to keep the code clean into the 
future...


The "offending" macros are all in petsclog.h which is included by 
petscsys.h.


thanks,

Eric