[cfe-commits] r165273 - in /cfe/trunk: lib/CodeGen/CodeGenFunction.cpp test/CodeGen/catch-undef-behavior.c test/CodeGenCXX/catch-undef-behavior.cpp test/CodeGenCXX/return.cpp

[Daniel Dunbar]

On Wed, Jan 23, 2013 at 5:18 PM, John McCall <rjmccall at apple.com> wrote:

> On Jan 23, 2013, at 4:51 PM, Daniel Dunbar <daniel at zuster.org> wrote:
>
> On Wed, Jan 23, 2013 at 4:43 PM, John McCall <rjmccall at apple.com> wrote:
>
>> On Jan 23, 2013, at 4:28 PM, Daniel Dunbar <daniel at zuster.org> wrote:
>>
>> On Wed, Jan 23, 2013 at 4:22 PM, John McCall <rjmccall at apple.com> wrote:
>>
>>> On Jan 23, 2013, at 4:08 PM, Daniel Dunbar <daniel at zuster.org> wrote:
>>>
>>> On Wed, Jan 23, 2013 at 2:17 PM, John McCall <rjmccall at apple.com> wrote:
>>>
>>>> On Jan 23, 2013, at 12:42 PM, Daniel Dunbar <daniel at zuster.org> wrote:
>>>>
>>>> On Wed, Jan 23, 2013 at 12:08 PM, John McCall <rjmccall at apple.com>wrote:
>>>>
>>>>> On Jan 23, 2013, at 11:57 AM, Daniel Dunbar <daniel at zuster.org> wrote:
>>>>>
>>>>> On Wed, Jan 23, 2013 at 11:44 AM, Chandler Carruth <
>>>>> chandlerc at google.com> wrote:
>>>>>
>>>>>>
>>>>>> On Wed, Jan 23, 2013 at 11:07 AM, John McCall <rjmccall at apple.com>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.
>>>
>>
>> Are you seriously proposing this is the "good thing"? I feel this is just
>> arguing to win not arguing for what is good for the user.
>>
>>
>> No.  The good thing is that the compiler can optimize somewhat better.
>>
>
> Are there convincing examples that this is really a useful optimization in
> general?
>
>
> Well, I've listed a whole bunch of situations that seem fairly convincing
> to me, but I've only described them in prose, so I guess I need to write
> them out so that you'll acknowledge them.
>
>   const char *getOrderString(NSComparisonResult result) {
>     switch (thing) {
>     case NSOrderedAscending: return "less than";
>     case NSOrderedSame: return "same as";
>     case NSOrderedDescending: return "greater than";
>     }
>   }
>
>   const char *describeThisInt(int x) {
>     if (x > 100) return "it's really big!";
>     if (x > 10) return "it's not that small";
>     if (x > 0) return "it's pretty small";
>     assert(0 && "A non-positive int?!  What is up with that???");
>     // ^ won't warn in debug builds on most platforms
>   }
>

Yes, but these seem like pretty limited examples of optimization. The
savings here are either a few instructions of presumed dead code or one
perfectly predicted branch.

 - Daniel

John.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130123/0c6cd09b/attachment.html>