Skip to content

Commit 86b3794

Browse files
committed
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 #97330.
1 parent 167c860 commit 86b3794

File tree

6 files changed

+60
-53
lines changed

6 files changed

+60
-53
lines changed

clang/test/CodeGen/inline-asm-x86-flag-output.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -380,10 +380,8 @@ int test_assume_boolean_flag(long nr, volatile long *addr) {
380380
//CHECK: %0 = tail call { i32, i32 } asm "cmp $2,$1", "={@cca},={@ccae},=*m,r,~{cc},~{dirflag},~{fpsr},~{flags}"(ptr elementtype(i64) %addr, i64 %nr)
381381
//CHECK: [[RES1:%.*]] = extractvalue { i32, i32 } %0, 0
382382
//CHECK: [[RES2:%.*]] = extractvalue { i32, i32 } %0, 1
383-
//CHECK: %1 = icmp ult i32 [[RES1]], 2
383+
//CHECK: %1 = icmp ult i32 [[RES2]], 2
384384
//CHECK: tail call void @llvm.assume(i1 %1)
385-
//CHECK: %2 = icmp ult i32 [[RES2]], 2
386-
//CHECK: tail call void @llvm.assume(i1 %2)
387385
int x,y;
388386
asm("cmp %2,%1"
389387
: "=@cca"(x), "=@ccae"(y), "=m"(*(volatile long *)(addr))

llvm/lib/Transforms/InstCombine/InstCombineInternal.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -545,10 +545,11 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final
545545
ConstantInt *&Less, ConstantInt *&Equal,
546546
ConstantInt *&Greater);
547547

548-
/// Attempts to replace V with a simpler value based on the demanded
548+
/// Attempts to replace I with a simpler value based on the demanded
549549
/// bits.
550-
Value *SimplifyDemandedUseBits(Value *V, APInt DemandedMask, KnownBits &Known,
551-
unsigned Depth, const SimplifyQuery &Q);
550+
Value *SimplifyDemandedUseBits(Instruction *I, const APInt &DemandedMask,
551+
KnownBits &Known, unsigned Depth,
552+
const SimplifyQuery &Q);
552553
using InstCombiner::SimplifyDemandedBits;
553554
bool SimplifyDemandedBits(Instruction *I, unsigned Op,
554555
const APInt &DemandedMask, KnownBits &Known,

llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp

Lines changed: 52 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,47 @@ bool InstCombinerImpl::SimplifyDemandedBits(Instruction *I, unsigned OpNo,
9191
KnownBits &Known, unsigned Depth,
9292
const SimplifyQuery &Q) {
9393
Use &U = I->getOperandUse(OpNo);
94-
Value *NewVal = SimplifyDemandedUseBits(U.get(), DemandedMask, Known,
95-
Depth, Q);
94+
Value *V = U.get();
95+
if (isa<Constant>(V)) {
96+
llvm::computeKnownBits(V, Known, Depth, Q);
97+
return false;
98+
}
99+
100+
Known.resetAll();
101+
if (DemandedMask.isZero()) {
102+
// Not demanding any bits from V.
103+
replaceUse(U, UndefValue::get(V->getType()));
104+
return true;
105+
}
106+
107+
if (Depth == MaxAnalysisRecursionDepth)
108+
return false;
109+
110+
Instruction *VInst = dyn_cast<Instruction>(V);
111+
if (!VInst) {
112+
llvm::computeKnownBits(V, Known, Depth, Q);
113+
return false;
114+
}
115+
116+
Value *NewVal;
117+
if (VInst->hasOneUse()) {
118+
// If the instruction has one use, we can directly simplify it.
119+
NewVal = SimplifyDemandedUseBits(VInst, DemandedMask, Known, Depth, Q);
120+
} else if (Depth != 0) {
121+
// If there are multiple uses of this instruction and we aren't at the root,
122+
// then we can simplify VInst to some other value, but not modify the
123+
// instruction.
124+
NewVal =
125+
SimplifyMultipleUseDemandedBits(VInst, DemandedMask, Known, Depth, Q);
126+
} else {
127+
// If this is the root being simplified, allow it to have multiple uses,
128+
// just set the DemandedMask to all bits and reset the context instruction.
129+
// This allows visitTruncInst (for example) to simplify the operand of a
130+
// trunc without duplicating all the SimplifyDemandedUseBits() logic.
131+
NewVal =
132+
SimplifyDemandedUseBits(VInst, APInt::getAllOnes(Known.getBitWidth()),
133+
Known, Depth, Q.getWithInstruction(VInst));
134+
}
96135
if (!NewVal) return false;
97136
if (Instruction* OpInst = dyn_cast<Instruction>(U))
98137
salvageDebugInfo(*OpInst);
@@ -124,50 +163,21 @@ bool InstCombinerImpl::SimplifyDemandedBits(Instruction *I, unsigned OpNo,
124163
/// operands based on the information about what bits are demanded. This returns
125164
/// some other non-null value if it found out that V is equal to another value
126165
/// in the context where the specified bits are demanded, but not for all users.
127-
Value *InstCombinerImpl::SimplifyDemandedUseBits(Value *V, APInt DemandedMask,
166+
Value *InstCombinerImpl::SimplifyDemandedUseBits(Instruction *I,
167+
const APInt &DemandedMask,
128168
KnownBits &Known,
129169
unsigned Depth,
130170
const SimplifyQuery &Q) {
131-
assert(V != nullptr && "Null pointer of Value???");
171+
assert(I != nullptr && "Null pointer of Value???");
132172
assert(Depth <= MaxAnalysisRecursionDepth && "Limit Search Depth");
133173
uint32_t BitWidth = DemandedMask.getBitWidth();
134-
Type *VTy = V->getType();
174+
Type *VTy = I->getType();
135175
assert(
136176
(!VTy->isIntOrIntVectorTy() || VTy->getScalarSizeInBits() == BitWidth) &&
137177
Known.getBitWidth() == BitWidth &&
138178
"Value *V, DemandedMask and Known must have same BitWidth");
139179

140-
if (isa<Constant>(V)) {
141-
llvm::computeKnownBits(V, Known, Depth, Q);
142-
return nullptr;
143-
}
144-
145-
Known.resetAll();
146-
if (DemandedMask.isZero()) // Not demanding any bits from V.
147-
return UndefValue::get(VTy);
148-
149-
if (Depth == MaxAnalysisRecursionDepth)
150-
return nullptr;
151-
152-
Instruction *I = dyn_cast<Instruction>(V);
153-
if (!I) {
154-
llvm::computeKnownBits(V, Known, Depth, Q);
155-
return nullptr; // Only analyze instructions.
156-
}
157-
158-
// If there are multiple uses of this value and we aren't at the root, then
159-
// we can't do any simplifications of the operands, because DemandedMask
160-
// only reflects the bits demanded by *one* of the users.
161-
if (Depth != 0 && !I->hasOneUse())
162-
return SimplifyMultipleUseDemandedBits(I, DemandedMask, Known, Depth, Q);
163-
164180
KnownBits LHSKnown(BitWidth), RHSKnown(BitWidth);
165-
// If this is the root being simplified, allow it to have multiple uses,
166-
// just set the DemandedMask to all bits so that we can try to simplify the
167-
// operands. This allows visitTruncInst (for example) to simplify the
168-
// operand of a trunc without duplicating all the logic below.
169-
if (Depth == 0 && !V->hasOneUse())
170-
DemandedMask.setAllBits();
171181

172182
// Update flags after simplifying an operand based on the fact that some high
173183
// order bits are not demanded.
@@ -1105,27 +1115,28 @@ Value *InstCombinerImpl::SimplifyDemandedUseBits(Value *V, APInt DemandedMask,
11051115
}
11061116

11071117
if (!KnownBitsComputed)
1108-
llvm::computeKnownBits(V, Known, Depth, Q);
1118+
llvm::computeKnownBits(I, Known, Depth, Q);
11091119
break;
11101120
}
11111121
}
11121122

1113-
if (V->getType()->isPointerTy()) {
1114-
Align Alignment = V->getPointerAlignment(DL);
1123+
if (I->getType()->isPointerTy()) {
1124+
Align Alignment = I->getPointerAlignment(DL);
11151125
Known.Zero.setLowBits(Log2(Alignment));
11161126
}
11171127

11181128
// If the client is only demanding bits that we know, return the known
11191129
// constant. We can't directly simplify pointers as a constant because of
11201130
// pointer provenance.
11211131
// TODO: We could return `(inttoptr const)` for pointers.
1122-
if (!V->getType()->isPointerTy() && DemandedMask.isSubsetOf(Known.Zero | Known.One))
1132+
if (!I->getType()->isPointerTy() &&
1133+
DemandedMask.isSubsetOf(Known.Zero | Known.One))
11231134
return Constant::getIntegerValue(VTy, Known.One);
11241135

11251136
if (VerifyKnownBits) {
1126-
KnownBits ReferenceKnown = llvm::computeKnownBits(V, Depth, Q);
1137+
KnownBits ReferenceKnown = llvm::computeKnownBits(I, Depth, Q);
11271138
if (Known != ReferenceKnown) {
1128-
errs() << "Mismatched known bits for " << *V << " in "
1139+
errs() << "Mismatched known bits for " << *I << " in "
11291140
<< I->getFunction()->getName() << "\n";
11301141
errs() << "computeKnownBits(): " << ReferenceKnown << "\n";
11311142
errs() << "SimplifyDemandedBits(): " << Known << "\n";

llvm/test/Transforms/InstCombine/assume-inseltpoison.ll

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@ define i32 @PR40940(<4 x i8> %x) {
1515
; CHECK-LABEL: @PR40940(
1616
; CHECK-NEXT: [[SHUF:%.*]] = shufflevector <4 x i8> [[X:%.*]], <4 x i8> poison, <4 x i32> <i32 1, i32 1, i32 2, i32 3>
1717
; CHECK-NEXT: [[T2:%.*]] = bitcast <4 x i8> [[SHUF]] to i32
18-
; CHECK-NEXT: [[T3:%.*]] = icmp ult i32 [[T2]], 65536
19-
; CHECK-NEXT: call void @llvm.assume(i1 [[T3]])
2018
; CHECK-NEXT: ret i32 [[T2]]
2119
;
2220
%shuf = shufflevector <4 x i8> %x, <4 x i8> poison, <4 x i32> <i32 1, i32 1, i32 2, i32 3>

llvm/test/Transforms/InstCombine/assume.ll

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -428,8 +428,6 @@ define i32 @PR40940(<4 x i8> %x) {
428428
; CHECK-LABEL: @PR40940(
429429
; CHECK-NEXT: [[SHUF:%.*]] = shufflevector <4 x i8> [[X:%.*]], <4 x i8> poison, <4 x i32> <i32 1, i32 1, i32 2, i32 3>
430430
; CHECK-NEXT: [[T2:%.*]] = bitcast <4 x i8> [[SHUF]] to i32
431-
; CHECK-NEXT: [[T3:%.*]] = icmp ult i32 [[T2]], 65536
432-
; CHECK-NEXT: call void @llvm.assume(i1 [[T3]])
433431
; CHECK-NEXT: ret i32 [[T2]]
434432
;
435433
%shuf = shufflevector <4 x i8> %x, <4 x i8> undef, <4 x i32> <i32 1, i32 1, i32 2, i32 3>

llvm/test/Transforms/InstCombine/known-bits.ll

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2025,15 +2025,16 @@ define i8 @simplifydemanded_context(i8 %x, i8 %y) {
20252025
ret i8 %and2
20262026
}
20272027

2028-
; FIXME: This is a miscompile.
20292028
define i16 @pr97330(i1 %c, ptr %p1, ptr %p2) {
20302029
; CHECK-LABEL: @pr97330(
20312030
; CHECK-NEXT: entry:
20322031
; CHECK-NEXT: br i1 [[C:%.*]], label [[EXIT:%.*]], label [[IF:%.*]]
20332032
; CHECK: if:
20342033
; CHECK-NEXT: unreachable
20352034
; CHECK: exit:
2036-
; CHECK-NEXT: ret i16 1
2035+
; CHECK-NEXT: [[V:%.*]] = load i64, ptr [[P1:%.*]], align 8
2036+
; CHECK-NEXT: [[CONV:%.*]] = trunc i64 [[V]] to i16
2037+
; CHECK-NEXT: ret i16 [[CONV]]
20372038
;
20382039
entry:
20392040
%v = load i64, ptr %p1, align 8

0 commit comments

Comments
 (0)