On Jan 23, 2013, at 5:18 PM, John McCall <[email protected]> wrote:
> On Jan 23, 2013, at 4:51 PM, Daniel Dunbar <[email protected]> wrote:
>> On Wed, Jan 23, 2013 at 4:43 PM, John McCall <[email protected]> wrote:
>> On Jan 23, 2013, at 4:28 PM, Daniel Dunbar <[email protected]> wrote:
>>> On Wed, Jan 23, 2013 at 4:22 PM, John McCall <[email protected]> wrote:
>>> 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.
>>>
>>> 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
> }
In a release build these will fall off the function after branches, so how
about emitting unreachable except when there was useful work in the block
before falling off, for example:
This gets 'unreachable':
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";
}
This does not get 'unreachable':
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";
reportThatWeMessedUpX(x);
// oops, forgot to return something
}
since by actually having code to execute the programmer's intention is not for
it to be unreachable.
-Argyrios_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
