[PATCH] D30667: GVNHoist: handle basic blocks with UnreachableInst

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added a comment.

In https://reviews.llvm.org/D30667#699749, @efriedma wrote:

> > Staring at other places it is used, it looks like it would also think it okay to propagate various non-nullness attributes, etc past unreachable.
>
> Propagate them to where?  unreachable doesn't have any successors.  This only makes sense if you connect the unreachable to the function's exit, or something like that.


Backwards:

  for (Instruction &I : Entry) {
     if (auto CS = CallSite(&I)) {
       if (auto *CalledFunc = CS.getCalledFunction()) {
         for (auto &CSArg : CalledFunc->args()) {
           if (!CSArg.hasNonNullAttr())
             continue;
  
           // If the non-null callsite argument operand is an argument to 'F'
           // (the caller) and the call is guaranteed to execute, then the value
           // must be non-null throughout 'F'.
           auto *FArg = dyn_cast<Argument>(CS.getArgOperand(CSArg.getArgNo()));
           if (FArg && !FArg->hasNonNullAttr()) {
             FArg->addAttr(Attribute::NonNull);
             Changed = true;
           }
         }
       }
     }
     if (!isGuaranteedToTransferExecutionToSuccessor(&I))
       break;
   }

IE the case i'm thinking of, but not sure if we care about:
If there are calls in the entry block, and the entry block ends in unreachable, we have the property that  the unreachable means we can assume the block doesn't execute.
But we are still using the calls  in the block pretending that they are guaranteed to execute.
:)

IE I was told the semantics of unreachable say that if we are guaranteed to hit the unreachable, along a given codepath, we can assume that code path cannot execute.
So should we be propagating non-nullness based on them?


Repository:
  rL LLVM

https://reviews.llvm.org/D30667