[PATCH] [codegen] Don't emit implicit unreachable at the end of a non-empty block

[Argyrios Kyrtzidis]

On Mar 17, 2013, at 4:51 PM, John McCall <rjmccall at apple.com> wrote:

> On Mar 16, 2013, at 10:14 AM, Argyrios Kyrtzidis <kyrtzidis at apple.com> wrote:
>> The attached patch adds a check so that, if we flow off the end of a value-returning function, the implicit unreachable will only be emitted if the returning block is empty.
>> For example:
>> 
>> int baz(int x) {
>> switch (x) {
>>   case 1: return 10;
>>   case 2: return 20;
>> }
>> // implicit unreachable is inserted
>> }
>> 
>> int baz2(int x) {
>> switch (x) {
>>   case 1: return 10;
>>   case 2: return 20;
>> }
>> foo();
>> // no unreachable
>> }
>> 
>> As a consequence, functions that have no returns, will not get the implicit unreachable:
>> 
>> int baz2(int x) {
>> foo();
>> // no unreachable
>> }
>> 
>> 
>> I believe this allows taking advantage of the optimization opportunities that the implicit unreachable intended, in a safer manner.
> 
> Hmm.  There's a pretty large spectrum of ways in which something could end up with a non-empty return block while still conceptually falling off the end in a way we'd like to optimize.

Could you give some examples ?

> We were talking about maybe re-using the same logic that we'd use for -Wreturn-type;

You mean, "don't put implicit unreachable if the warning would be triggered" ?

> what's the value in doing this intermediate stage?

Compared to "using" -Wreturn-type, I think checking the flowing-off-the-end block is much simpler and:

-We won't need to start always computing the CFG
- -Wreturn-type does not seem to fit all the cases that are applicable for the unreachable optimization, for example -Wreturn-type will be triggered for this:

int no_return2(int x) {
  if (x > 10)
    return 2;
  if (x > 0)
    return 1;
 // assert(0) invisible in release build
}


> 
> Also, if we do decide to do this, please don't use empty();  that can lead to us generating semantically different code based on the presence of debug info.  I believe you can instead ask whether getFirstNonPHIOrDbgOrLifetime() returns null.

Thanks for letting me know.

> 
> John.
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130318/05383d7d/attachment.html>