On Feb 1, 2013, at 5:04 PM, Argyrios Kyrtzidis <[email protected]> wrote:
> 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 > } > Another way of saying the same thing is to not make function 'unreachable' if it has side-effect. - Fariborz > 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
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
