Silence warnings about no-op alignment operations

[markj]

Summary

In these cases the page size and firmware page size are the same, and
clang warns that the roundup operation is a no-op
(-Wtautological-compare). I can't see a way to fix this other than to
conditionally compile the code in question, or to disable the
diagnostic. Suggestions for alternate solutions are welcome.

BTW it is kind of perplexing to me that clang warns about a no-op here,
but the same expression performs a right-shift by zero and we don't get
a warning about that. Perhaps clang should be more accomodating here,
perhaps by providing a different warning flag? -Wtautological-compare
is generally useful I think.

[arichardson]

That warning is my fault, possibly clang should have a -Wtautological-compare-align warning flag to turn this off.
I think using the pragma is the only way to silence it right now.

The only other option I can think of is an inline function wrapper:
https://godbolt.org/z/1cYc5G

[markj]

In D28576#639718, @arichardson wrote:

That warning is my fault, possibly clang should have a -Wtautological-compare-align warning flag to turn this off.

It's conceivable that this warning could be useful, but it obviously isn't here given the comments.

I think using the pragma is the only way to silence it right now.

The only other option I can think of is an inline function wrapper:
https://godbolt.org/z/1cYc5G

I have no real preference either way. Using pragmas is a bit more surgical and I think I prefer that since I can't easily test these drivers.

[arichardson]

It should probably be #pragma clang, I'm not sure GCC implements -Wtautological-compare (and it won't warn since it doesn't support the builtin yet).

[markj]

In D28576#639730, @arichardson wrote:

It should probably be #pragma clang, I'm not sure GCC implements -Wtautological-compare (and it won't warn since it doesn't support the builtin yet).

Hmm, the amd64 xtoolchain gcc fails to build the kernel with a related error:

/usr/home/markj/src/freebsd-dev/sys/dev/firewire/fwohci.c:2699:17: error: 'typeof' applied to a bit-field                                                                                                                                                                                                                     
   r += roundup2(fp->mode.wreqb.len, sizeof(uint32_t));                                                                                                                                                                                                                                                                       
                 ^                                                                                                                                                                                                                                                                                                            
/usr/home/markj/src/freebsd-dev/sys/sys/cdefs.h:895:15: note: in definition of macro '__builtin_align_up'                                                                                                                                                                                                                     
  ((__typeof__(x))(((__uintptr_t)(x)+((align)-1))&(~((align)-1))))                                                                                             
               ^                                                                                                                                                                                                                                                                                                              
/usr/home/markj/src/freebsd-dev/sys/sys/param.h:310:24: note: in expansion of macro '__align_up'                                                                                                                                                                                                                              
 #define roundup2(x, y) __align_up(x, y) /* if y is powers of two */

[arichardson]

@jhb noticed that too, and @jrtc27 suggested __typeof__(x+0) might be a possible workaround for GCC.

[jrtc27]

If these are preprocessor-time constants you could do #if MTHCA_ICM_PAGE_SIZE < PAGE_SIZE instead.

[markj]

In D28576#639736, @jrtc27 wrote:

If these are preprocessor-time constants you could do #if MTHCA_ICM_PAGE_SIZE < PAGE_SIZE instead.

*_ICM_PAGE_SIZE is an enum. Initially I converted them to preprocessor constants but just decided to use the pragma since the prevailing style in these drivers is to use enum to define constants.

[markj]

In D28576#639740, @markj wrote:

the prevailing style in these drivers is to use enum to define constants.

Upon a closer look I guess that's not true. Ok, will change.

[markj]

Conditionally compile the offending code.

[arichardson]

sys/dev/mthca/mthca_cmd.c

1594

[markj]

Fix up one other place I missed.

[• hselasky]

Looks good.