Page MenuHomeFreeBSD

Silence warnings about no-op alignment operations
ClosedPublic

Authored by markj on Wed, Feb 10, 5:23 PM.

Details

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.

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

markj requested review of this revision.Wed, Feb 10, 5:23 PM

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

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.

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).

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 */

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

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

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.

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.

Conditionally compile the offending code.

sys/dev/mthca/mthca_cmd.c
1592

Fix up one other place I missed.

This revision is now accepted and ready to land.Wed, Feb 10, 8:33 PM