[PATCH util-macros 1/2] Don't disable strict aliasing (-fno-strict-aliasing) globally

[Jeremy Huddleston]

On Nov 1, 2010, at 13:18, Gaetan Nadon wrote:

> On Mon, 2010-11-01 at 12:33 -0700, Jeremy Huddleston wrote:
> 
>> On Nov 1, 2010, at 05:48, Gaetan Nadon wrote:
>> 
>>> On Mon, 2010-11-01 at 11:32 +0100, Mark Kettenis wrote:
>>> 
>>>> I may be somewhat overcautious, but I would keep -fno-strict-aliasing
>>>> as a default.  And I'd only enable -fstrict-aliasing for particular
>>>> bits of code where it has a significant performance benefit, and
>>>> people have done a careful analysis of the code to see if it is free
>>>> of aliasing issues.
>>> 
>>> 
>>> The cautious approach is the only one that will get consensus.
>>> Here is a proposal:
>>> 
>>> 
>>>    1. Separate the aliasing flag from the warning flags as outlined in
>>>       a previous post. This is prep work, status quo is preserved. In
>>>       addition it prevents adding aliasing flag to modules that
>>>       currently don't have it without their knowledge or consent.
>> 
>> So we would create two new macros:
>> 
>> XORG_CFLAGS_WARNINGS would set CFLAGS_WARNINGS="-Wall -Wformat -W..."
> 
> Yes, contains only warning flags, nothing else.
> 
>> XORG_CFLAGS_NO_STRICT_ALIASING would set CFLAGS_NO_STRICT_ALIASING="-fno-strict-aliasing"
> 
> It would be called CFLAGS_STRICT_ALIASING with -fstrict-aliasing, under
> an "opt-in" principle.

I don't think that's the correct model.  -fstrict-aliasing is on by default (for many users), and -fno-strict-aliasing should be what is opted into in order to disable optimizations that rely on correctly written C code.

>> XORG_CWARNFLAGS would be updated to call these two and set CWARNFLAGS="$(CFLAGS_WARNINGS) $(CFLAGS_NO_STRICT_ALIASING)"
> 
> Nope. Our good old CWARNFLAGS would remain untouched for eternity and
> will eventually fall off the radar screen.

Well, I think that in the spirit of abstraction and code-reuse, it would be best to have it call the two new macros... but I guess that is contingent on our dispute over opting-in to -fno-strict-aliasing versus -fstrict-aliasing.

...

>> For modules that did have it historically, we'll leave them alone initially.  
>> As we audit them, we'll change CWARNFLAGS to either CFLAGS_WARNINGS or CFLAGS_WARNINGS CFLAGS_NO_STRICT_ALIASI.  
>> This will help us keep track of what has been audited to determine what really needs 
>> the flag versus what might've inherited it by accident.
> 
> Those modules would use both the new CFLAGS_WARNINGS and the new
> CFLAGS_STRICT_ALIASING, under an opt-in principle

I'm confused now.  If they actually *NEED* -fno-strict-aliasing, how are you proposing adding -fno-strict-aliasing?  This is why I was suggesting a CFLAGS_NO_STRICT_ALIASING