[clang] 86b3794 - Reapply [InstCombine] Fix context for multi-use demanded bits simplification
Nikita Popov via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 2 02:03:24 PDT 2024
Author: Nikita Popov
Date: 2024-07-02T11:02:55+02:00
New Revision: 86b37944a70229b07626e63bdb9a46b4bc3d1460
URL: https://github.com/llvm/llvm-project/commit/86b37944a70229b07626e63bdb9a46b4bc3d1460
DIFF: https://github.com/llvm/llvm-project/commit/86b37944a70229b07626e63bdb9a46b4bc3d1460.diff
LOG: Reapply [InstCombine] Fix context for multi-use demanded bits simplification
Repplied with a clang test fix.
-----
When simplifying a multi-use root value, the demanded bits were
reset to full, but we also need to reset the context instruction.
To make this convenient (without requiring by-value passing of
SimplifyQuery), move the logic that handles constants and
dispatches to SimplifyDemandedUseBits/SimplifyMultipleUseDemandedBits
into SimplifyDemandedBits. The SimplifyDemandedInstructionBits
caller starts with full demanded bits and an appropriate context
anyway.
The different context instruction does mean that the ephemeral
value protection no longer triggers in some cases, as the changes
to assume tests show.
An alternative, which I will explore in a followup, is to always
use SimplifyMultipleUseDemandedBits() -- the previous root special
case is only really intended for SimplifyDemandedInstructionBits(),
which now no longer shares this code path.
Fixes https://github.com/llvm/llvm-project/issues/97330.
Added:
Modified:
clang/test/CodeGen/inline-asm-x86-flag-output.c
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/clang/test/CodeGen/inline-asm-x86-flag-output.c b/clang/test/CodeGen/inline-asm-x86-flag-output.c
index 243dc3716ca13..7ca6dc7a39f50 100644
--- a/clang/test/CodeGen/inline-asm-x86-flag-output.c
+++ b/clang/test/CodeGen/inline-asm-x86-flag-output.c
@@ -380,10 +380,8 @@ int test_assume_boolean_flag(long nr, volatile long *addr) {
//CHECK: %0 = tail call { i32, i32 } asm "cmp $2,$1", "={@cca},={@ccae},=*m,r,~{cc},~{dirflag},~{fpsr},~{flags}"(ptr elementtype(i64) %addr, i64 %nr)
//CHECK: [[RES1:%.*]] = extractvalue { i32, i32 } %0, 0
//CHECK: [[RES2:%.*]] = extractvalue { i32, i32 } %0, 1
- //CHECK: %1 = icmp ult i32 [[RES1]], 2
+ //CHECK: %1 = icmp ult i32 [[RES2]], 2
//CHECK: tail call void @llvm.assume(i1 %1)
- //CHECK: %2 = icmp ult i32 [[RES2]], 2
- //CHECK: tail call void @llvm.assume(i1 %2)
int x,y;
asm("cmp %2,%1"
: "=@cca"(x), "=@ccae"(y), "=m"(*(volatile long *)(addr))
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
index 318c455fd7ef1..64fbcc80e0edf 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
+++ b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
@@ -545,10 +545,11 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final
ConstantInt *&Less, ConstantInt *&Equal,
ConstantInt *&Greater);
- /// Attempts to replace V with a simpler value based on the demanded
+ /// Attempts to replace I with a simpler value based on the demanded
/// bits.
- Value *SimplifyDemandedUseBits(Value *V, APInt DemandedMask, KnownBits &Known,
- unsigned Depth, const SimplifyQuery &Q);
+ Value *SimplifyDemandedUseBits(Instruction *I, const 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 6cf2e71363ab5..b1d03786f3218 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
@@ -91,8 +91,47 @@ bool InstCombinerImpl::SimplifyDemandedBits(Instruction *I, unsigned OpNo,
KnownBits &Known, unsigned Depth,
const SimplifyQuery &Q) {
Use &U = I->getOperandUse(OpNo);
- Value *NewVal = SimplifyDemandedUseBits(U.get(), DemandedMask, Known,
- Depth, Q);
+ 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));
+ }
if (!NewVal) return false;
if (Instruction* OpInst = dyn_cast<Instruction>(U))
salvageDebugInfo(*OpInst);
@@ -124,50 +163,21 @@ 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(Value *V, APInt DemandedMask,
+Value *InstCombinerImpl::SimplifyDemandedUseBits(Instruction *I,
+ const APInt &DemandedMask,
KnownBits &Known,
unsigned Depth,
const SimplifyQuery &Q) {
- assert(V != nullptr && "Null pointer of Value???");
+ assert(I != nullptr && "Null pointer of Value???");
assert(Depth <= MaxAnalysisRecursionDepth && "Limit Search Depth");
uint32_t BitWidth = DemandedMask.getBitWidth();
- Type *VTy = V->getType();
+ Type *VTy = I->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.
@@ -1105,13 +1115,13 @@ Value *InstCombinerImpl::SimplifyDemandedUseBits(Value *V, APInt DemandedMask,
}
if (!KnownBitsComputed)
- llvm::computeKnownBits(V, Known, Depth, Q);
+ llvm::computeKnownBits(I, Known, Depth, Q);
break;
}
}
- if (V->getType()->isPointerTy()) {
- Align Alignment = V->getPointerAlignment(DL);
+ if (I->getType()->isPointerTy()) {
+ Align Alignment = I->getPointerAlignment(DL);
Known.Zero.setLowBits(Log2(Alignment));
}
@@ -1119,13 +1129,14 @@ Value *InstCombinerImpl::SimplifyDemandedUseBits(Value *V, APInt DemandedMask,
// constant. We can't directly simplify pointers as a constant because of
// pointer provenance.
// TODO: We could return `(inttoptr const)` for pointers.
- if (!V->getType()->isPointerTy() && DemandedMask.isSubsetOf(Known.Zero | Known.One))
+ if (!I->getType()->isPointerTy() &&
+ DemandedMask.isSubsetOf(Known.Zero | Known.One))
return Constant::getIntegerValue(VTy, Known.One);
if (VerifyKnownBits) {
- KnownBits ReferenceKnown = llvm::computeKnownBits(V, Depth, Q);
+ KnownBits ReferenceKnown = llvm::computeKnownBits(I, Depth, Q);
if (Known != ReferenceKnown) {
- errs() << "Mismatched known bits for " << *V << " in "
+ errs() << "Mismatched known bits for " << *I << " 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 79a1ec2c1efc2..a03ff3e4c89f0 100644
--- a/llvm/test/Transforms/InstCombine/assume-inseltpoison.ll
+++ b/llvm/test/Transforms/InstCombine/assume-inseltpoison.ll
@@ -15,8 +15,6 @@ 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 474da9968b66a..2ef7433e60324 100644
--- a/llvm/test/Transforms/InstCombine/assume.ll
+++ b/llvm/test/Transforms/InstCombine/assume.ll
@@ -428,8 +428,6 @@ 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 8520981f63f3e..c7445a6ce2fe2 100644
--- a/llvm/test/Transforms/InstCombine/known-bits.ll
+++ b/llvm/test/Transforms/InstCombine/known-bits.ll
@@ -2025,7 +2025,6 @@ 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:
@@ -2033,7 +2032,9 @@ define i16 @pr97330(i1 %c, ptr %p1, ptr %p2) {
; CHECK: if:
; CHECK-NEXT: unreachable
; CHECK: exit:
-; CHECK-NEXT: ret i16 1
+; CHECK-NEXT: [[V:%.*]] = load i64, ptr [[P1:%.*]], align 8
+; CHECK-NEXT: [[CONV:%.*]] = trunc i64 [[V]] to i16
+; CHECK-NEXT: ret i16 [[CONV]]
;
entry:
%v = load i64, ptr %p1, align 8
More information about the cfe-commits
mailing list