[SCCP] Split up callsite handling, only propagate result on change (NFC)

[fhahn]

Summary

Functions include their arguments in the use-list. Changed function
values mean that the result of the function changed. We only need
to update the call sites with the new function result and do not
have to propagate the call arguments.

To do so, this patch splits up the visitCallSite into handleCallResult
and handleCallArguments and updates markUsersAsChanged to only update
call results for functions.

[efriedma]

This makes sense, I think. No testcase because you can't test it on its own?

[fhahn]

In D75846#1917389, @efriedma wrote:

This makes sense, I think. No testcase because you can't test it on its own?

Yes, this change should be NFC, as we should only skip redundant updates without constant ranges.

[fhahn]

In D75846#1919428, @fhahn wrote:

In D75846#1917389, @efriedma wrote:

This makes sense, I think. No testcase because you can't test it on its own?

Yes, this change should be NFC, as we should only skip redundant updates without constant ranges.

I've updated the title to reflect that. There are also no changes in the binaries when building MultiSource, SPEC2000 & SPEC2006 with and without the patch.

[fhahn]

Rebased after recent changes landed.

[efriedma]

LGTM

llvm/lib/Transforms/Scalar/SCCP.cpp

private:
   // Add U as additional user of V.
   void addAdditionalUser(Value *V, User *U) {
     auto Iter = AdditionalUsers.insert({V, {}});
     Iter.first->second.insert(U);
   }
 
   // Mark I's users as changed, including AdditionalUsers.
   void markUsersAsChanged(Value *I) {
+    // Functions include their arguments in the use-list. Changed function values mean that the result of the function changed. We only need to update the call sites with the new function result and do not have to propagate the call arguments.
+    if (auto *Fn = dyn_cast<Function>(I)) {
+      for (User *U : I->users()) {
+        if (auto CS = CallSite(U))
+          handleCallResult(CS);
+      }
+    } else {
       for (User *U : I->users())
         if (auto *UI = dyn_cast<Instruction>(U))
           OperandChangedState(UI);
+    }
 
     auto Iter = AdditionalUsers.find(I);
     if (Iter != AdditionalUsers.end()) {
       for (User *U : Iter->second)
         if (auto *UI = dyn_cast<Instruction>(U))
           OperandChangedState(UI);
     }
   }
+  void handleCallOverdefined(CallSite CS);
+  void handleCallResult(CallSite CS);
+  void handleCallArguments(CallSite CS);
 
 private:
   friend class InstVisitor<SCCPSolver>;
 
   // visit implementations - Something changed in this instruction.  Either an
   // operand made a transition, or the instruction is newly executable.  Change
   // the value type of I to reflect these changes if appropriate.
   void visitPHINode(PHINode &I);
void SCCPSolver::visitLoadInst(LoadInst &I) {
   }
 
   // Otherwise we cannot say for certain what value this load will produce.
   // Bail out.
   markOverdefined(IV, &I);
 }
 
 void SCCPSolver::visitCallSite(CallSite CS) {
-  Function *F = CS.getCalledFunction();
-  Instruction *I = CS.getInstruction();
-
-  if (auto *II = dyn_cast<IntrinsicInst>(I)) {
-    if (II->getIntrinsicID() == Intrinsic::ssa_copy) {
-      if (ValueState[I].isOverdefined())
-        return;
-
-      auto *PI = getPredicateInfoFor(I);
-      if (!PI)
-        return;
-
-      Value *CopyOf = I->getOperand(0);
-      auto *PBranch = dyn_cast<PredicateBranch>(PI);
-      if (!PBranch) {
-        mergeInValue(ValueState[I], I, getValueState(CopyOf));
-        return;
-      }
+  handleCallResult(CS);
+  handleCallArguments(CS);
 
-      Value *Cond = PBranch->Condition;
-
-      // Everything below relies on the condition being a comparison.
-      auto *Cmp = dyn_cast<CmpInst>(Cond);
-      if (!Cmp) {
-        mergeInValue(ValueState[I], I, getValueState(CopyOf));
-        return;
-      }
-
-      Value *CmpOp0 = Cmp->getOperand(0);
-      Value *CmpOp1 = Cmp->getOperand(1);
-      if (CopyOf != CmpOp0 && CopyOf != CmpOp1) {
-        mergeInValue(ValueState[I], I, getValueState(CopyOf));
-        return;
-      }
-
-      if (CmpOp0 != CopyOf)
-        std::swap(CmpOp0, CmpOp1);
-
-      LatticeVal OriginalVal = getValueState(CopyOf);
-      LatticeVal EqVal = getValueState(CmpOp1);
-      LatticeVal &IV = ValueState[I];
-      if (PBranch->TrueEdge && Cmp->getPredicate() == CmpInst::ICMP_EQ) {
-        addAdditionalUser(CmpOp1, I);
-        if (OriginalVal.isConstant())
-          mergeInValue(IV, I, OriginalVal);
-        else
-          mergeInValue(IV, I, EqVal);
-        return;
-      }
-      if (!PBranch->TrueEdge && Cmp->getPredicate() == CmpInst::ICMP_NE) {
-        addAdditionalUser(CmpOp1, I);
-        if (OriginalVal.isConstant())
-          mergeInValue(IV, I, OriginalVal);
-        else
-          mergeInValue(IV, I, EqVal);
-        return;
-      }
-
-      return (void)mergeInValue(IV, I, getValueState(CopyOf));
-    }
-  }
+void SCCPSolver::handleCallOverdefined(CallSite CS) {
+  Function *F = CS.getCalledFunction();
+  Instruction *I = CS.getInstruction();
 
-  // The common case is that we aren't tracking the callee, either because we
-  // are not doing interprocedural analysis or the callee is indirect, or is
-  // external.  Handle these cases first.
-  if (!F || F->isDeclaration()) {
-CallOverdefined:
-    // Void return and not tracking callee, just bail.
-    if (I->getType()->isVoidTy()) return;
+  if (I->getType()->isVoidTy())
+    return;
 
-    // Otherwise, if we have a single return value case, and if the function is
-    // a declaration, maybe we can constant fold it.
-    if (F && F->isDeclaration() && !I->getType()->isStructTy() &&
-        canConstantFoldCallTo(cast<CallBase>(CS.getInstruction()), F)) {
-      SmallVector<Constant*, 8> Operands;
-      for (CallSite::arg_iterator AI = CS.arg_begin(), E = CS.arg_end();
-           AI != E; ++AI) {
-        if (AI->get()->getType()->isStructTy())
-          return markOverdefined(I); // Can't handle struct args.
-        LatticeVal State = getValueState(*AI);
+    SmallVector<Constant *, 8> Operands;
+    for (CallSite::arg_iterator AI = CS.arg_begin(), E = CS.arg_end(); AI != E;
+         ++AI) {
 
-        if (State.isUnknown())
-          return;  // Operands are not resolved yet.
-        if (State.isOverdefined())
-          return (void)markOverdefined(I);
-        assert(State.isConstant() && "Unknown state!");
-        Operands.push_back(State.getConstant());
-      }
+        return; // Operands are not resolved yet.
 
-      if (getValueState(I).isOverdefined())
-        return;
 
-      // If we can constant fold this, mark the result of the call as a
-      // constant.
-      if (Constant *C = ConstantFoldCall(cast<CallBase>(CS.getInstruction()), F,
-                                         Operands, &GetTLI(*F))) {
-        // call -> undef.
-        if (isa<UndefValue>(C))
-          return;
-        return (void)markConstant(I, C);
-      }
-    }
 
-    // Otherwise, we don't know anything about this call, mark it overdefined.
-    return (void)markOverdefined(I);
-  }
 
+void SCCPSolver::handleCallArguments(CallSite CS) {
+  Function *F = CS.getCalledFunction();
   // If this is a local function that doesn't have its address taken, mark its
   // entry block executable and merge in the actual arguments to the call into
   // the formal arguments of the function.
   if (!TrackingIncomingArguments.empty() && TrackingIncomingArguments.count(F)){
     MarkBlockExecutable(&F->front());
 
     // Propagate information from this call site into the callee.
     CallSite::arg_iterator CAI = CS.arg_begin();
     for (Function::arg_iterator AI = F->arg_begin(), E = F->arg_end();
          AI != E; ++AI, ++CAI) {
       // If this argument is byval, and if the function is not readonly, there
       // will be an implicit copy formed of the input aggregate.
       if (AI->hasByValAttr() && !F->onlyReadsMemory()) {
         markOverdefined(&*AI);
         continue;
       }
 
for (Function::arg_iterator AI = F->arg_begin(), E = F->arg_end();
         // ValueState does not change (because it is already overdefined there),
         // We have to take changes in ParamState into account, as it is used
         // when evaluating Cmp instructions.
         if (!ValueChanged && ParamChanged)
           pushToWorkList(ValueState[&*AI], &*AI);
       }
     }
   }
+}
+
+void SCCPSolver::handleCallResult(CallSite CS) {
+  Function *F = CS.getCalledFunction();
+  Instruction *I = CS.getInstruction();
+
+  if (auto *II = dyn_cast<IntrinsicInst>(I)) {
+    if (II->getIntrinsicID() == Intrinsic::ssa_copy) {
+      if (ValueState[I].isOverdefined())
+        return;
+
+      auto *PI = getPredicateInfoFor(I);
+      if (!PI)
+        return;
+
+      Value *CopyOf = I->getOperand(0);
+      auto *PBranch = dyn_cast<PredicateBranch>(PI);
+      if (!PBranch) {
+        mergeInValue(ValueState[I], I, getValueState(CopyOf));
+        return;
+      }
+
+      Value *Cond = PBranch->Condition;
+
+      // Everything below relies on the condition being a comparison.
+      auto *Cmp = dyn_cast<CmpInst>(Cond);
+      if (!Cmp) {
+        mergeInValue(ValueState[I], I, getValueState(CopyOf));
+        return;
+      }
+
+      Value *CmpOp0 = Cmp->getOperand(0);
+      Value *CmpOp1 = Cmp->getOperand(1);
+      if (CopyOf != CmpOp0 && CopyOf != CmpOp1) {
+        mergeInValue(ValueState[I], I, getValueState(CopyOf));
+        return;
+      }
+
+      if (CmpOp0 != CopyOf)
+        std::swap(CmpOp0, CmpOp1);
+
+      LatticeVal OriginalVal = getValueState(CopyOf);
+      LatticeVal EqVal = getValueState(CmpOp1);
+      LatticeVal &IV = ValueState[I];
+      if (PBranch->TrueEdge && Cmp->getPredicate() == CmpInst::ICMP_EQ) {
+        addAdditionalUser(CmpOp1, I);
+        if (OriginalVal.isConstant())
+          mergeInValue(IV, I, OriginalVal);
+        else
+          mergeInValue(IV, I, EqVal);
+        return;
+      }
+      if (!PBranch->TrueEdge && Cmp->getPredicate() == CmpInst::ICMP_NE) {
+        addAdditionalUser(CmpOp1, I);
+        if (OriginalVal.isConstant())
+          mergeInValue(IV, I, OriginalVal);
+        else
+          mergeInValue(IV, I, EqVal);
+        return;
+      }
+
+      return (void)mergeInValue(IV, I, getValueState(CopyOf));
+    }
+  }
+
+  // The common case is that we aren't tracking the callee, either because we
+  // are not doing interprocedural analysis or the callee is indirect, or is
+  // external.  Handle these cases first.
+  if (!F || F->isDeclaration())
+    return handleCallOverdefined(CS);
 
   // If this is a single/zero retval case, see if we're tracking the function.
   if (auto *STy = dyn_cast<StructType>(F->getReturnType())) {
     if (!MRVFunctionsTracked.count(F))
-      goto CallOverdefined;  // Not tracking this callee.
+      return handleCallOverdefined(CS); // Not tracking this callee.
 
     // If we are tracking this callee, propagate the result of the function
     // into this call site.
     for (unsigned i = 0, e = STy->getNumElements(); i != e; ++i)
       mergeInValue(getStructValueState(I, i), I,
                    TrackedMultipleRetVals[std::make_pair(F, i)]);
   } else {
     MapVector<Function*, LatticeVal>::iterator TFRVI = TrackedRetVals.find(F);
     if (TFRVI == TrackedRetVals.end())
-      goto CallOverdefined;  // Not tracking this callee.
+      return handleCallOverdefined(CS); // Not tracking this callee.
 
     // If so, propagate the return value of the callee into this call result.
     mergeInValue(I, TFRVI->second);
   }
 }
 
 void SCCPSolver::Solve() {
   // Process the work lists until they are empty!