[IPSCCP] Move callsite check to the beginning of the loop.

[fhahn]

Summary

We have some code marks instructions with struct operands as overdefined,
but if the instruction is a call to a function with tracked arguments,
this breaks the assumption that the lattice values of all call sites
are not overdefined and will be replaced by a constant.

This also re-adds the assertion from D65222, with additionally skipping
non-callsite uses. This patch should address the cases reported in which
the assertion fired.

Fixes PR42738.

[efriedma]

LGTM

llvm/trunk/lib/Transforms/Scalar/SCCP.cpp

for (Instruction &I : BB) {
           LatticeVal &LV = getStructValueState(&I, i);
           if (LV.isUnknown())
             markOverdefined(LV, &I);
         }
         continue;
       }
 
       LatticeVal &LV = getValueState(&I);
-      if (!LV.isUnknown()) continue;
+      if (!LV.isUnknown())
+        continue;
+
+      // There are two reasons a call can have an undef result
+      // 1. It could be tracked.
+      // 2. It could be constant-foldable.
+      // Because of the way we solve return values, tracked calls must
+      // never be marked overdefined in ResolvedUndefsIn.
+      if (CallSite CS = CallSite(&I)) {
+        if (Function *F = CS.getCalledFunction())
+          if (TrackedRetVals.count(F))
+            continue;
+
+        // If the call is constant-foldable, we mark it overdefined because
+        // we do not know what return values are valid.
+        markOverdefined(&I);
+        return true;
+      }
 
       // extractvalue is safe; check here because the argument is a struct.
       if (isa<ExtractValueInst>(I))
         continue;
 
       // Compute the operand LatticeVals, for convenience below.
       // Anything taking a struct is conservatively assumed to require
       // overdefined markings.
for (Instruction &I : BB) {
         if ((Op0LV.isUnknown() || Op1LV.isUnknown()) &&
             cast<ICmpInst>(&I)->isEquality())
           break;
         markOverdefined(&I);
         return true;
       case Instruction::Call:
       case Instruction::Invoke:
       case Instruction::CallBr:
-        // There are two reasons a call can have an undef result
-        // 1. It could be tracked.
-        // 2. It could be constant-foldable.
-        // Because of the way we solve return values, tracked calls must
-        // never be marked overdefined in ResolvedUndefsIn.
-        if (Function *F = CallSite(&I).getCalledFunction())
-          if (TrackedRetVals.count(F))
-            break;
-
-        // If the call is constant-foldable, we mark it overdefined because
-        // we do not know what return values are valid.
-        markOverdefined(&I);
-        return true;
+        llvm_unreachable("Call-like instructions should have be handled early");
       default:
         // If we don't know what should happen here, conservatively mark it
         // overdefined.
         markOverdefined(&I);
         return true;
       }
     }
 
static void findReturnsToZap(Function &F,
   // There is a non-removable musttail call site of this function. Zapping
   // returns is not allowed.
   if (Solver.isMustTailCallee(&F)) {
     LLVM_DEBUG(dbgs() << "Can't zap returns of the function : " << F.getName()
                       << " due to present musttail call of it\n");
     return;
   }
 
+  assert(
+      all_of(F.users(),
+             [&Solver](User *U) {
+               if (isa<Instruction>(U) &&
+                   !Solver.isBlockExecutable(cast<Instruction>(U)->getParent()))
+                 return true;
+               // Non-callsite uses are not impacted by zapping. Also, constant
+               // uses (like blockaddresses) could stuck around, without being
+               // used in the underlying IR, meaning we do not have lattice
+               // values for them.
+               if (!CallSite(U))
+                 return true;
+               if (U->getType()->isStructTy()) {
+                 return all_of(
+                     Solver.getStructLatticeValueFor(U),
+                     [](const LatticeVal &LV) { return !LV.isOverdefined(); });
+               }
+               return !Solver.getLatticeValueFor(U).isOverdefined();
+             }) &&
+      "We can only zap functions where all live users have a concrete value");
+
   for (BasicBlock &BB : F) {
     if (CallInst *CI = BB.getTerminatingMustTailCall()) {
       LLVM_DEBUG(dbgs() << "Can't zap return of the block due to present "
                         << "musttail call : " << *CI << "\n");
       (void)CI;
       return;
     }
 

llvm/trunk/test/Transforms/SCCP/indirectbr.ll

-; RUN: opt -S -sccp < %s | FileCheck %s
+; RUN: opt -S -ipsccp < %s | FileCheck %s
 
 declare void @BB0_f()
 declare void @BB1_f()
 
 ; Make sure we can eliminate what is in BB0 as we know that the indirectbr is going to BB1.
 ;
 ; CHECK-LABEL: define void @indbrtest1(
 ; CHECK-NOT: call void @BB0_f()
BB0:
   call void @BB0_f()
   br label %BB1
 BB1:
   call void @BB1_f()
   ret void
 }
 
 
+; CHECK-LABEL: define internal i32 @indbrtest5(
+; CHECK: ret i32 undef
+define internal i32 @indbrtest5(i1 %c) {
+entry:
+  br i1 %c, label %bb1, label %bb2
+
+bb1:
+  br label %branch.block
+
+
+bb2:
+  br label %branch.block
+
+branch.block:
+  %addr = phi i8* [blockaddress(@indbrtest5, %target1), %bb1], [blockaddress(@indbrtest5, %target2), %bb2]
+  indirectbr i8* %addr, [label %target1, label %target2]
+
+target1:
+  br label %target2
+
+target2:
+  ret i32 10
+}
+
+
+define i32 @indbrtest5_callee(i1 %c) {
+; CHECK-LABEL: define i32 @indbrtest5_callee(
+; CHECK-NEXT:    %r = call i32 @indbrtest5(i1 %c)
+; CHECK-NEXT:    ret i32 10
+  %r = call i32 @indbrtest5(i1 %c)
+  ret i32 %r
+}

llvm/trunk/test/Transforms/SCCP/struct-arg-resolve-undefs.ll

+; RUN: opt -ipsccp -S %s | FileCheck %s
+
+%struct.S = type { i32 }
+
+
+define void @main() {
+; CHECK-LABEL: void @main() {
+; CHECK-NEXT:    %r = call i32 @f(%struct.S { i32 100 })
+; CHECK-NEXT:    call void @do_report(i32 123)
+  %r = call i32 @f(%struct.S { i32 100 })
+  call void @do_report(i32 %r)
+  ret void
+}
+
+declare void @do_report(i32)
+
+define internal i32 @f(%struct.S %s.coerce) {
+; CHECK-LABEL: define internal i32 @f(%struct.S %s.coerce)
+; CHECK-LABEL: entry:
+; CHECK-NEXT:     %call = call i8 @lsh(i8 1, i32 100)
+; CHECK-LABEL: if.end:
+; CHECK-NEXT:     ret i32 undef
+entry:
+  %ev = extractvalue %struct.S %s.coerce, 0
+  %call = call i8 @lsh(i8 1, i32 %ev)
+  %tobool = icmp ne i8 %call, 0
+  br i1 %tobool, label %for.cond, label %if.end
+
+for.cond:                                         ; preds = %for.cond, %if.then
+  %i.0 = phi i32 [ 0, %entry], [ %inc, %for.cond ]
+  %cmp = icmp slt i32 %i.0, 1
+  %inc = add nsw i32 %i.0, 1
+  br i1 %cmp, label %for.cond, label %if.end
+
+if.end:                                           ; preds = %for.cond, %entry
+  ret i32 123
+}
+
+define internal i8 @lsh(i8 %l, i32 %r) {
+entry:
+  %conv = sext i8 %l to i32
+  %cmp = icmp slt i32 %conv, 0
+  %shr = ashr i32 127, %r
+  %cmp4 = icmp sgt i32 %conv, %shr
+  %or.cond13 = or i1 %cmp, %cmp4
+  %cond = select i1 %or.cond13, i32 %conv, i32 0
+  %conv7 = trunc i32 %cond to i8
+  ret i8 %conv7
+}