[SCCP] Don't check for UndefValue before calling markConstant()

[nikic]

Summary

The value lattice explicitly represents undef, and markConstant() internally checks for UndefValue and will create an undef rather than constant lattice element in that case.

This is mostly a code simplification, it has little practical impact because we usually get undef results from undef operands, and those don't get processed.

Only leave the check behind for the CmpInst case, because it currently goes through this incorrect code in the getCompare() implementation: https://github.com/llvm/llvm-project/blob/f98697642cea761448dc0f84f750d3f5def8af6b/llvm/include/llvm/Analysis/ValueLattice.h#L456-L457

[nikic]

Rebase

[nikic]

Ping

[fhahn]

LGTM, thanks!

llvm/lib/Transforms/Utils/SCCPSolver.cpp

ValueLatticeElement &getStructValueState(Value *V, unsigned i) {
     if (!I.second)
       return LV; // Common case, already in the map.
 
     if (auto *C = dyn_cast<Constant>(V)) {
       Constant *Elt = C->getAggregateElement(i);
 
       if (!Elt)
         LV.markOverdefined(); // Unknown sort of constant.
-      else if (isa<UndefValue>(Elt))
-        ; // Undef values remain unknown.
       else
         LV.markConstant(Elt); // Constants are constant.
     }
 
     // All others are underdefined by default.
     return LV;
   }
 
SCCPInstVisitor(const DataLayout &DL,
                   std::function<const TargetLibraryInfo &(Function &)> GetTLI,
                   LLVMContext &Ctx)
       : DL(DL), GetTLI(GetTLI), Ctx(Ctx) {}
 
   void trackValueOfGlobalVariable(GlobalVariable *GV) {
     // We only track the contents of scalar globals.
     if (GV->getValueType()->isSingleValueType()) {
       ValueLatticeElement &IV = TrackedGlobals[GV];
-      if (!isa<UndefValue>(GV->getInitializer()))
-        IV.markConstant(GV->getInitializer());
     }
   }
 
   void addTrackedFunction(Function *F) {
     // Add an entry, F -> undef.
     if (auto *STy = dyn_cast<StructType>(F->getReturnType())) {
       MRVFunctionsTracked.insert(F);
       for (unsigned i = 0, e = STy->getNumElements(); i != e; ++i)
void SCCPInstVisitor::visitCastInst(CastInst &I) {
 
   ValueLatticeElement OpSt = getValueState(I.getOperand(0));
   if (OpSt.isUnknownOrUndef())
     return;
 
   if (Constant *OpC = getConstant(OpSt)) {
     // Fold the constant as we build.
     Constant *C = ConstantFoldCastOperand(I.getOpcode(), OpC, I.getType(), DL);
-    if (isa<UndefValue>(C))
-      return;
-    // Propagate constant value
     markConstant(&I, C);
   } else if (I.getDestTy()->isIntegerTy()) {
     auto &LV = getValueState(&I);
     ConstantRange OpRange =
         OpSt.isConstantRange()
             ? OpSt.getConstantRange()
             : ConstantRange::getFull(
                   I.getOperand(0)->getType()->getScalarSizeInBits());
void SCCPInstVisitor::visitUnaryOperator(Instruction &I) {
   ValueLatticeElement V0State = getValueState(I.getOperand(0));
 
   ValueLatticeElement &IV = ValueState[&I];
   // resolvedUndefsIn might mark I as overdefined. Bail out, even if we would
   // discover a concrete value later.
   if (isOverdefined(IV))
     return (void)markOverdefined(&I);
 
-  if (isConstant(V0State)) {
+  if (isConstant(V0State))
     if (Constant *C = ConstantFoldUnaryOpOperand(I.getOpcode(),
-                                                 getConstant(V0State), DL)) {
-      // op Y -> undef.
-      if (isa<UndefValue>(C))
-        return;
+                                                 getConstant(V0State), DL))
       return (void)markConstant(IV, &I, C);
-    }
-  }
 
   // If something is undef, wait for it to resolve.
   if (!isOverdefined(V0State))
     return;
 
   markOverdefined(&I);
 }
 
void SCCPInstVisitor::visitBinaryOperator(Instruction &I) {
   // If either of the operands is a constant, try to fold it to a constant.
   // TODO: Use information from notconstant better.
   if ((V1State.isConstant() || V2State.isConstant())) {
     Value *V1 = isConstant(V1State) ? getConstant(V1State) : I.getOperand(0);
     Value *V2 = isConstant(V2State) ? getConstant(V2State) : I.getOperand(1);
     Value *R = simplifyBinOp(I.getOpcode(), V1, V2, SimplifyQuery(DL));
     auto *C = dyn_cast_or_null<Constant>(R);
     if (C) {
-      // X op Y -> undef.
-      if (isa<UndefValue>(C))
-        return;
       // Conservatively assume that the result may be based on operands that may
       // be undef. Note that we use mergeInValue to combine the constant with
       // the existing lattice value for I, as different constants might be found
       // after one of the operands go to overdefined, e.g. due to one operand
       // being a special floating value.
       ValueLatticeElement NewV;
       NewV.markConstant(C, /*MayIncludeUndef=*/true);
       return (void)mergeInValue(&I, NewV);
void SCCPInstVisitor::visitCmpInst(CmpInst &I) {
 
   // For parameters, use ParamState which includes constant range info if
   // available.
   auto V1State = getValueState(Op1);
   auto V2State = getValueState(Op2);
 
   Constant *C = V1State.getCompare(I.getPredicate(), I.getType(), V2State);
   if (C) {
+    // TODO: getCompare() currently has incorrect handling for unknown/undef.
     if (isa<UndefValue>(C))
       return;
     ValueLatticeElement CV;
     CV.markConstant(C);
     mergeInValue(&I, CV);
     return;
   }
 
for (unsigned i = 0, e = I.getNumOperands(); i != e; ++i) {
 
     return (void)markOverdefined(&I);
   }
 
   Constant *Ptr = Operands[0];
   auto Indices = makeArrayRef(Operands.begin() + 1, Operands.end());
   Constant *C =
       ConstantExpr::getGetElementPtr(I.getSourceElementType(), Ptr, Indices);
-  if (isa<UndefValue>(C))
-    return;
   markConstant(&I, C);
 }
 
 void SCCPInstVisitor::visitStoreInst(StoreInst &SI) {
   // If this store is of a struct, ignore it.
   if (SI.getOperand(0)->getType()->isStructTy())
     return;
 
if (auto *GV = dyn_cast<GlobalVariable>(Ptr)) {
         if (It != TrackedGlobals.end()) {
           mergeInValue(IV, &I, It->second, getMaxWidenStepsOpts());
           return;
         }
       }
     }
 
     // Transform load from a constant into a constant if possible.
-    if (Constant *C = ConstantFoldLoadFromConstPtr(Ptr, I.getType(), DL)) {
-      if (isa<UndefValue>(C))
-        return;
+    if (Constant *C = ConstantFoldLoadFromConstPtr(Ptr, I.getType(), DL))
       return (void)markConstant(IV, &I, C);
-    }
-  }
 
   // Fall back to metadata.
   mergeInValue(&I, getValueFromMetadata(&I));
 }
 
 void SCCPInstVisitor::visitCallBase(CallBase &CB) {
   handleCallResult(CB);
   handleCallArguments(CB);
for (const Use &A : CB.args()) {
       Operands.push_back(getConstant(State));
     }
 
     if (isOverdefined(getValueState(&CB)))
       return (void)markOverdefined(&CB);
 
     // If we can constant fold this, mark the result of the call as a
     // constant.
-    if (Constant *C = ConstantFoldCall(&CB, F, Operands, &GetTLI(*F))) {
-      // call -> undef.
-      if (isa<UndefValue>(C))
-        return;
+    if (Constant *C = ConstantFoldCall(&CB, F, Operands, &GetTLI(*F)))
       return (void)markConstant(&CB, C);
-    }
-  }
 
   // Fall back to metadata.
   mergeInValue(&CB, getValueFromMetadata(&CB));
 }
 
 void SCCPInstVisitor::handleCallArguments(CallBase &CB) {
   Function *F = CB.getCalledFunction();
   // If this is a local function that doesn't have its address taken, mark its

llvm/test/Transforms/SCCP/undef-resolve.ll

;
   %t = icmp ugt i32 undef, -1
   ret i1 %t
 }
 
 ; Make sure we handle extractvalue
 define i64 @test10() {
 ; CHECK-LABEL: @test10(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[E:%.*]] = extractvalue { i64, i64 } undef, 1
-; CHECK-NEXT:    ret i64 [[E]]
+; CHECK-NEXT:    ret i64 undef
 ;
 entry:
   %e = extractvalue { i64, i64 } undef, 1
   ret i64 %e
 }
 
 @GV = common global i32 0, align 4