[llvm] 167c860 - Revert "[InstCombine] Fix context for multi-use demanded bits simplification"

Nikita Popov via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 2 01:44:42 PDT 2024


Author: Nikita Popov
Date: 2024-07-02T10:44:34+02:00
New Revision: 167c860ba209fc584f72d04c48091b40ae3a5610

URL: https://github.com/llvm/llvm-project/commit/167c860ba209fc584f72d04c48091b40ae3a5610
DIFF: https://github.com/llvm/llvm-project/commit/167c860ba209fc584f72d04c48091b40ae3a5610.diff

LOG: Revert "[InstCombine] Fix context for multi-use demanded bits simplification"

This reverts commit b558ac0eef57a3737b1e27844115fa91e0b32582.

This breaks a clang test, reverting for now.

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineInternal.h
    llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
    llvm/test/Transforms/InstCombine/assume-inseltpoison.ll
    llvm/test/Transforms/InstCombine/assume.ll
    llvm/test/Transforms/InstCombine/known-bits.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
index 64fbcc80e0edf..318c455fd7ef1 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
+++ b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
@@ -545,11 +545,10 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final
                                ConstantInt *&Less, ConstantInt *&Equal,
                                ConstantInt *&Greater);
 
-  /// Attempts to replace I with a simpler value based on the demanded
+  /// Attempts to replace V with a simpler value based on the demanded
   /// bits.
-  Value *SimplifyDemandedUseBits(Instruction *I, const APInt &DemandedMask,
-                                 KnownBits &Known, unsigned Depth,
-                                 const SimplifyQuery &Q);
+  Value *SimplifyDemandedUseBits(Value *V, APInt DemandedMask, KnownBits &Known,
+                                 unsigned Depth, const SimplifyQuery &Q);
   using InstCombiner::SimplifyDemandedBits;
   bool SimplifyDemandedBits(Instruction *I, unsigned Op,
                             const APInt &DemandedMask, KnownBits &Known,

diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
index b1d03786f3218..6cf2e71363ab5 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
@@ -91,47 +91,8 @@ bool InstCombinerImpl::SimplifyDemandedBits(Instruction *I, unsigned OpNo,
                                             KnownBits &Known, unsigned Depth,
                                             const SimplifyQuery &Q) {
   Use &U = I->getOperandUse(OpNo);
-  Value *V = U.get();
-  if (isa<Constant>(V)) {
-    llvm::computeKnownBits(V, Known, Depth, Q);
-    return false;
-  }
-
-  Known.resetAll();
-  if (DemandedMask.isZero()) {
-    // Not demanding any bits from V.
-    replaceUse(U, UndefValue::get(V->getType()));
-    return true;
-  }
-
-  if (Depth == MaxAnalysisRecursionDepth)
-    return false;
-
-  Instruction *VInst = dyn_cast<Instruction>(V);
-  if (!VInst) {
-    llvm::computeKnownBits(V, Known, Depth, Q);
-    return false;
-  }
-
-  Value *NewVal;
-  if (VInst->hasOneUse()) {
-    // If the instruction has one use, we can directly simplify it.
-    NewVal = SimplifyDemandedUseBits(VInst, DemandedMask, Known, Depth, Q);
-  } else if (Depth != 0) {
-    // If there are multiple uses of this instruction and we aren't at the root,
-    // then we can simplify VInst to some other value, but not modify the
-    // instruction.
-    NewVal =
-        SimplifyMultipleUseDemandedBits(VInst, DemandedMask, Known, Depth, Q);
-  } else {
-    // If this is the root being simplified, allow it to have multiple uses,
-    // just set the DemandedMask to all bits and reset the context instruction.
-    // This allows visitTruncInst (for example) to simplify the operand of a
-    // trunc without duplicating all the SimplifyDemandedUseBits() logic.
-    NewVal =
-        SimplifyDemandedUseBits(VInst, APInt::getAllOnes(Known.getBitWidth()),
-                                Known, Depth, Q.getWithInstruction(VInst));
-  }
+  Value *NewVal = SimplifyDemandedUseBits(U.get(), DemandedMask, Known,
+                                          Depth, Q);
   if (!NewVal) return false;
   if (Instruction* OpInst = dyn_cast<Instruction>(U))
     salvageDebugInfo(*OpInst);
@@ -163,21 +124,50 @@ bool InstCombinerImpl::SimplifyDemandedBits(Instruction *I, unsigned OpNo,
 /// operands based on the information about what bits are demanded. This returns
 /// some other non-null value if it found out that V is equal to another value
 /// in the context where the specified bits are demanded, but not for all users.
-Value *InstCombinerImpl::SimplifyDemandedUseBits(Instruction *I,
-                                                 const APInt &DemandedMask,
+Value *InstCombinerImpl::SimplifyDemandedUseBits(Value *V, APInt DemandedMask,
                                                  KnownBits &Known,
                                                  unsigned Depth,
                                                  const SimplifyQuery &Q) {
-  assert(I != nullptr && "Null pointer of Value???");
+  assert(V != nullptr && "Null pointer of Value???");
   assert(Depth <= MaxAnalysisRecursionDepth && "Limit Search Depth");
   uint32_t BitWidth = DemandedMask.getBitWidth();
-  Type *VTy = I->getType();
+  Type *VTy = V->getType();
   assert(
       (!VTy->isIntOrIntVectorTy() || VTy->getScalarSizeInBits() == BitWidth) &&
       Known.getBitWidth() == BitWidth &&
       "Value *V, DemandedMask and Known must have same BitWidth");
 
+  if (isa<Constant>(V)) {
+    llvm::computeKnownBits(V, Known, Depth, Q);
+    return nullptr;
+  }
+
+  Known.resetAll();
+  if (DemandedMask.isZero()) // Not demanding any bits from V.
+    return UndefValue::get(VTy);
+
+  if (Depth == MaxAnalysisRecursionDepth)
+    return nullptr;
+
+  Instruction *I = dyn_cast<Instruction>(V);
+  if (!I) {
+    llvm::computeKnownBits(V, Known, Depth, Q);
+    return nullptr;        // Only analyze instructions.
+  }
+
+  // If there are multiple uses of this value and we aren't at the root, then
+  // we can't do any simplifications of the operands, because DemandedMask
+  // only reflects the bits demanded by *one* of the users.
+  if (Depth != 0 && !I->hasOneUse())
+    return SimplifyMultipleUseDemandedBits(I, DemandedMask, Known, Depth, Q);
+
   KnownBits LHSKnown(BitWidth), RHSKnown(BitWidth);
+  // If this is the root being simplified, allow it to have multiple uses,
+  // just set the DemandedMask to all bits so that we can try to simplify the
+  // operands.  This allows visitTruncInst (for example) to simplify the
+  // operand of a trunc without duplicating all the logic below.
+  if (Depth == 0 && !V->hasOneUse())
+    DemandedMask.setAllBits();
 
   // Update flags after simplifying an operand based on the fact that some high
   // order bits are not demanded.
@@ -1115,13 +1105,13 @@ Value *InstCombinerImpl::SimplifyDemandedUseBits(Instruction *I,
     }
 
     if (!KnownBitsComputed)
-      llvm::computeKnownBits(I, Known, Depth, Q);
+      llvm::computeKnownBits(V, Known, Depth, Q);
     break;
   }
   }
 
-  if (I->getType()->isPointerTy()) {
-    Align Alignment = I->getPointerAlignment(DL);
+  if (V->getType()->isPointerTy()) {
+    Align Alignment = V->getPointerAlignment(DL);
     Known.Zero.setLowBits(Log2(Alignment));
   }
 
@@ -1129,14 +1119,13 @@ Value *InstCombinerImpl::SimplifyDemandedUseBits(Instruction *I,
   // constant. We can't directly simplify pointers as a constant because of
   // pointer provenance.
   // TODO: We could return `(inttoptr const)` for pointers.
-  if (!I->getType()->isPointerTy() &&
-      DemandedMask.isSubsetOf(Known.Zero | Known.One))
+  if (!V->getType()->isPointerTy() && DemandedMask.isSubsetOf(Known.Zero | Known.One))
     return Constant::getIntegerValue(VTy, Known.One);
 
   if (VerifyKnownBits) {
-    KnownBits ReferenceKnown = llvm::computeKnownBits(I, Depth, Q);
+    KnownBits ReferenceKnown = llvm::computeKnownBits(V, Depth, Q);
     if (Known != ReferenceKnown) {
-      errs() << "Mismatched known bits for " << *I << " in "
+      errs() << "Mismatched known bits for " << *V << " in "
              << I->getFunction()->getName() << "\n";
       errs() << "computeKnownBits(): " << ReferenceKnown << "\n";
       errs() << "SimplifyDemandedBits(): " << Known << "\n";

diff  --git a/llvm/test/Transforms/InstCombine/assume-inseltpoison.ll b/llvm/test/Transforms/InstCombine/assume-inseltpoison.ll
index a03ff3e4c89f0..79a1ec2c1efc2 100644
--- a/llvm/test/Transforms/InstCombine/assume-inseltpoison.ll
+++ b/llvm/test/Transforms/InstCombine/assume-inseltpoison.ll
@@ -15,6 +15,8 @@ define i32 @PR40940(<4 x i8> %x) {
 ; CHECK-LABEL: @PR40940(
 ; CHECK-NEXT:    [[SHUF:%.*]] = shufflevector <4 x i8> [[X:%.*]], <4 x i8> poison, <4 x i32> <i32 1, i32 1, i32 2, i32 3>
 ; CHECK-NEXT:    [[T2:%.*]] = bitcast <4 x i8> [[SHUF]] to i32
+; CHECK-NEXT:    [[T3:%.*]] = icmp ult i32 [[T2]], 65536
+; CHECK-NEXT:    call void @llvm.assume(i1 [[T3]])
 ; CHECK-NEXT:    ret i32 [[T2]]
 ;
   %shuf = shufflevector <4 x i8> %x, <4 x i8> poison, <4 x i32> <i32 1, i32 1, i32 2, i32 3>

diff  --git a/llvm/test/Transforms/InstCombine/assume.ll b/llvm/test/Transforms/InstCombine/assume.ll
index 2ef7433e60324..474da9968b66a 100644
--- a/llvm/test/Transforms/InstCombine/assume.ll
+++ b/llvm/test/Transforms/InstCombine/assume.ll
@@ -428,6 +428,8 @@ define i32 @PR40940(<4 x i8> %x) {
 ; CHECK-LABEL: @PR40940(
 ; CHECK-NEXT:    [[SHUF:%.*]] = shufflevector <4 x i8> [[X:%.*]], <4 x i8> poison, <4 x i32> <i32 1, i32 1, i32 2, i32 3>
 ; CHECK-NEXT:    [[T2:%.*]] = bitcast <4 x i8> [[SHUF]] to i32
+; CHECK-NEXT:    [[T3:%.*]] = icmp ult i32 [[T2]], 65536
+; CHECK-NEXT:    call void @llvm.assume(i1 [[T3]])
 ; CHECK-NEXT:    ret i32 [[T2]]
 ;
   %shuf = shufflevector <4 x i8> %x, <4 x i8> undef, <4 x i32> <i32 1, i32 1, i32 2, i32 3>

diff  --git a/llvm/test/Transforms/InstCombine/known-bits.ll b/llvm/test/Transforms/InstCombine/known-bits.ll
index c7445a6ce2fe2..8520981f63f3e 100644
--- a/llvm/test/Transforms/InstCombine/known-bits.ll
+++ b/llvm/test/Transforms/InstCombine/known-bits.ll
@@ -2025,6 +2025,7 @@ define i8 @simplifydemanded_context(i8 %x, i8 %y) {
   ret i8 %and2
 }
 
+; FIXME: This is a miscompile.
 define i16 @pr97330(i1 %c, ptr %p1, ptr %p2) {
 ; CHECK-LABEL: @pr97330(
 ; CHECK-NEXT:  entry:
@@ -2032,9 +2033,7 @@ define i16 @pr97330(i1 %c, ptr %p1, ptr %p2) {
 ; CHECK:       if:
 ; CHECK-NEXT:    unreachable
 ; CHECK:       exit:
-; CHECK-NEXT:    [[V:%.*]] = load i64, ptr [[P1:%.*]], align 8
-; CHECK-NEXT:    [[CONV:%.*]] = trunc i64 [[V]] to i16
-; CHECK-NEXT:    ret i16 [[CONV]]
+; CHECK-NEXT:    ret i16 1
 ;
 entry:
   %v = load i64, ptr %p1, align 8


        


More information about the llvm-commits mailing list