From 6db3edc8588c036680a8696e1735ffc52cb57156 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov@redhat.com>
Date: Wed, 22 Jun 2022 11:27:58 +0200
Subject: [PATCH] [SCCP] Don't check for UndefValue before calling
 markConstant()

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

Differential Revision: https://reviews.llvm.org/D128330
---
 llvm/lib/Transforms/Utils/SCCPSolver.cpp   | 34 ++++------------------
 llvm/test/Transforms/SCCP/undef-resolve.ll |  3 +-
 2 files changed, 7 insertions(+), 30 deletions(-)

diff --git a/llvm/lib/Transforms/Utils/SCCPSolver.cpp b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
index 871ce69ef024..3cec437ea33d 100644
--- a/llvm/lib/Transforms/Utils/SCCPSolver.cpp
+++ b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
@@ -208,8 +208,6 @@ private:
 
       if (!Elt)
         LV.markOverdefined(); // Unknown sort of constant.
-      else if (isa<UndefValue>(Elt))
-        ; // Undef values remain unknown.
       else
         LV.markConstant(Elt); // Constants are constant.
     }
@@ -356,8 +354,7 @@ public:
     // 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());
+      IV.markConstant(GV->getInitializer());
     }
   }
 
@@ -822,9 +819,6 @@ void SCCPInstVisitor::visitCastInst(CastInst &I) {
   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);
@@ -959,15 +953,10 @@ void SCCPInstVisitor::visitUnaryOperator(Instruction &I) {
   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))
@@ -1000,9 +989,6 @@ void SCCPInstVisitor::visitBinaryOperator(Instruction &I) {
     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
@@ -1051,6 +1037,7 @@ void SCCPInstVisitor::visitCmpInst(CmpInst &I) {
 
   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;
@@ -1096,8 +1083,6 @@ void SCCPInstVisitor::visitGetElementPtrInst(GetElementPtrInst &I) {
   auto Indices = makeArrayRef(Operands.begin() + 1, Operands.end());
   Constant *C =
       ConstantExpr::getGetElementPtr(I.getSourceElementType(), Ptr, Indices);
-  if (isa<UndefValue>(C))
-    return;
   markConstant(&I, C);
 }
 
@@ -1175,11 +1160,8 @@ void SCCPInstVisitor::visitLoadInst(LoadInst &I) {
     }
 
     // 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.
@@ -1224,12 +1206,8 @@ void SCCPInstVisitor::handleCallOverdefined(CallBase &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.
diff --git a/llvm/test/Transforms/SCCP/undef-resolve.ll b/llvm/test/Transforms/SCCP/undef-resolve.ll
index 1216ae1773e8..f66978e08a23 100644
--- a/llvm/test/Transforms/SCCP/undef-resolve.ll
+++ b/llvm/test/Transforms/SCCP/undef-resolve.ll
@@ -246,8 +246,7 @@ define i1 @test9() {
 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