On Jan 23, 2013, at 4:08 PM, Daniel Dunbar <[email protected]> wrote:
> On Wed, Jan 23, 2013 at 2:17 PM, John McCall <[email protected]> wrote:
> On Jan 23, 2013, at 12:42 PM, Daniel Dunbar <[email protected]> wrote:
>> On Wed, Jan 23, 2013 at 12:08 PM, John McCall <[email protected]> wrote:
>> On Jan 23, 2013, at 11:57 AM, Daniel Dunbar <[email protected]> wrote:
>>> On Wed, Jan 23, 2013 at 11:44 AM, Chandler Carruth <[email protected]>
>>> wrote:
>>>
>>> On Wed, Jan 23, 2013 at 11:07 AM, John McCall <[email protected]> wrote:
>>> A significant part of the problem, I believe, is that there's a lot of
>>> mostly-externally-maintained C code which, at Apple, happens to need to be
>>> compiled as C++.
>>>
>>> FWIW, this makes perfect sense, and also makes perfect sense out of a flag
>>> to essentially get C's return semantics in a C++ compilation in order to
>>> support such code.
>>>
>>> This is still the wrong direction of the flag. I just haven't seen good
>>> justification for changing the compiler in this way to merit the
>>> possibility of breaking working code.
>>
>> Every change can break working code. Warning changes can break working code
>> if it's compiled with -Werror. "Show me a whole-percentage speedup or take
>> the optimization out" is not really a reasonable response to every last
>> proposal.
>>
>> Yes, but that doesn't mean such changes should be made without consideration
>> either. My argument is that I do not think there is sufficient user benefit
>> to motivate this change.
>>
>> In LLVM and clang, we have a lot of places where we use unreachable
>> annotations; I think Chandler's argument is quite correct that these
>> situations come up all the time for many users and that it's ultimately not
>> reasonable to expect non-compiler people to use those annotations
>> pervasively.
>>
>> Our specific internal problem that makes this seem like a non-starter is
>> that we have a pool of known code that's very awkward to fix. We do control
>> the build environment for that code, though. For purposes of investigation,
>> we can reasonably assume that any project that turns off -Wreturn-value
>> should probably also disable the optimization. Any stragglers can be
>> tracked down and fixed just like we would with any other compiler change.
>>
>> You are hijacking my argument. My opinion doesn't have anything to do with
>> an internal problem, I just happen to think this is the wrong choice for
>> users.
>>
>> In my opinion, for *most* users and most code it is more important that the
>> code work than that it be optimal. I think this is the kind of optimization
>> that compiler hackers and low-level optimization people might find very
>> desirable, but anyone writing code that depended on it should still be using
>> an attribute or other marker.
>>
>> Again in my opinion, for most users, the compiler is just a tool they use to
>> get work done. They like it to optimize, and they like it to give nice
>> warnings, but overall they want it to help them get work done and not force
>> them to change their code.
>
> I do not think that this is a reasonable standard by which we can judge
> optimizations. The compiler is a tool. Like most tools, it can do good
> things but is capable of mischief. Like most tools, users come to take the
> good things for granted, but they notice the mischief immediately. It's
> wrong to forsake the good just because it's less visible; you have to
> actually make a thoughtful decision to balance these things, and that means
> not immediately throwing out all the good the first time you see the bad.
>
> By these standards, what is the good that this change is making?
>
> I stand by what I said before -- I still have not seen justification for
> changing the compiler in this way to merit the possibility of breaking
> working code.
>
> The only justification that has been offered so far is that this change can
> help the compiler optimize somewhat better ***only in the case of code that
> would emit a compiler warning***.
A user can audit their code, decide that the end of a function is in practice
unreachable, and take appropriate measures — they might disable that warning,
either file-wide or with a #pragma, or they might simply choose to ignore it,
knowing that it's a false positive. For example, they might have an
assert(false) there and just ignore warnings in release mode. The
optimal-for-clang solution of adding an explicit call to
__builtin_unreachable() is not necessarily acceptable to all users; the most
obvious flaw is that it's not portable (e.g. it was new in GCC 4.5).
John.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
