Skip to content

Commit 1848525

Browse files
committed
[CodeMetrics] Don't require speculatability for ephemeral values
As discussed in D112016, our current requirement of speculatability for ephemeral is overly strict: What we really care about is that the instruction will be DCEd once the assume is dropped. For that it is sufficient that the instruction is side-effect free and not a terminator. In particular, this allows non-dereferenceable loads to be ephemeral values. Differential Revision: https://reviews.llvm.org/D112179
1 parent f7587a9 commit 1848525

File tree

5 files changed

+31
-23
lines changed

5 files changed

+31
-23
lines changed

llvm/lib/Analysis/CodeMetrics.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,9 @@ appendSpeculatableOperands(const Value *V,
3434

3535
for (const Value *Operand : U->operands())
3636
if (Visited.insert(Operand).second)
37-
if (isSafeToSpeculativelyExecute(Operand))
38-
Worklist.push_back(Operand);
37+
if (const auto *I = dyn_cast<Instruction>(Operand))
38+
if (!I->mayHaveSideEffects() && !I->isTerminator())
39+
Worklist.push_back(I);
3940
}
4041

4142
static void completeEphemeralValues(SmallPtrSetImpl<const Value *> &Visited,

llvm/lib/Analysis/ValueTracking.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,9 @@ static bool isEphemeralValueOf(const Instruction *I, const Value *E) {
494494
if (V == E)
495495
return true;
496496

497-
if (V == I || isSafeToSpeculativelyExecute(V)) {
497+
if (V == I || (isa<Instruction>(V) &&
498+
!cast<Instruction>(V)->mayHaveSideEffects() &&
499+
!cast<Instruction>(V)->isTerminator())) {
498500
EphValues.insert(V);
499501
if (const User *U = dyn_cast<User>(V))
500502
append_range(WorkSet, U->operands());

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2577,11 +2577,11 @@ static bool BlockIsSimpleEnoughToThreadThrough(BasicBlock *BB) {
25772577
int Size = 0;
25782578

25792579
SmallPtrSet<const Value *, 32> EphValues;
2580-
auto IsEphemeral = [&](const Value *V) {
2581-
if (isa<AssumeInst>(V))
2580+
auto IsEphemeral = [&](const Instruction *I) {
2581+
if (isa<AssumeInst>(I))
25822582
return true;
2583-
return isSafeToSpeculativelyExecute(V) &&
2584-
all_of(V->users(),
2583+
return !I->mayHaveSideEffects() && !I->isTerminator() &&
2584+
all_of(I->users(),
25852585
[&](const User *U) { return EphValues.count(U); });
25862586
};
25872587

llvm/test/Transforms/Inline/ephemeral.ll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,10 @@ define i32 @inner(i8* %y) {
2626
ret i32 %a1
2727
}
2828

29-
; TODO: The load should be considered ephemeral here, even though it is not
30-
; speculatable.
29+
; Only the ret should be included in the instruction count, the load and icmp
30+
; are both ephemeral.
3131
; CHECK: Analyzing call of inner2...
32-
; CHECK: NumInstructions: 2
32+
; CHECK: NumInstructions: 1
3333
define void @inner2(i8* %y) {
3434
%v = load i8, i8* %y
3535
%c = icmp eq i8 %v, 42

llvm/test/Transforms/SimplifyCFG/unprofitable-pr.ll

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -233,32 +233,37 @@ false2: ; preds = %true1
233233
ret void
234234
}
235235

236-
; TODO: The load should be considered ephemeral here, even though it is not
237-
; speculatable.
236+
; The load, icmp and assume should not count towards the limit, they are
237+
; ephemeral.
238238
define void @test_non_speculatable(i1 %c, i64* align 1 %ptr, i8* %ptr2) local_unnamed_addr #0 {
239239
; CHECK-LABEL: @test_non_speculatable(
240-
; CHECK-NEXT: br i1 [[C:%.*]], label [[TRUE1:%.*]], label [[FALSE1:%.*]]
241-
; CHECK: true1:
240+
; CHECK-NEXT: br i1 [[C:%.*]], label [[TRUE2_CRITEDGE:%.*]], label [[FALSE1:%.*]]
241+
; CHECK: false1:
242+
; CHECK-NEXT: store volatile i64 1, i64* [[PTR:%.*]], align 4
242243
; CHECK-NEXT: [[V:%.*]] = load i8, i8* [[PTR2:%.*]], align 1
243244
; CHECK-NEXT: [[C2:%.*]] = icmp eq i8 [[V]], 42
244245
; CHECK-NEXT: call void @llvm.assume(i1 [[C2]])
245-
; CHECK-NEXT: store volatile i64 0, i64* [[PTR:%.*]], align 8
246+
; CHECK-NEXT: store volatile i64 0, i64* [[PTR]], align 8
246247
; CHECK-NEXT: store volatile i64 -1, i64* [[PTR]], align 8
247248
; CHECK-NEXT: store volatile i64 -1, i64* [[PTR]], align 8
248249
; CHECK-NEXT: store volatile i64 -1, i64* [[PTR]], align 8
249250
; CHECK-NEXT: store volatile i64 -1, i64* [[PTR]], align 8
250251
; CHECK-NEXT: store volatile i64 -1, i64* [[PTR]], align 8
251-
; CHECK-NEXT: br i1 [[C]], label [[TRUE2:%.*]], label [[FALSE2:%.*]]
252-
; CHECK: false1:
253-
; CHECK-NEXT: store volatile i64 1, i64* [[PTR]], align 4
254-
; CHECK-NEXT: br label [[TRUE1]]
252+
; CHECK-NEXT: store volatile i64 3, i64* [[PTR]], align 8
253+
; CHECK-NEXT: br label [[COMMON_RET:%.*]]
255254
; CHECK: common.ret:
256255
; CHECK-NEXT: ret void
257-
; CHECK: true2:
256+
; CHECK: true2.critedge:
257+
; CHECK-NEXT: [[V_C:%.*]] = load i8, i8* [[PTR2]], align 1
258+
; CHECK-NEXT: [[C2_C:%.*]] = icmp eq i8 [[V_C]], 42
259+
; CHECK-NEXT: call void @llvm.assume(i1 [[C2_C]])
260+
; CHECK-NEXT: store volatile i64 0, i64* [[PTR]], align 8
261+
; CHECK-NEXT: store volatile i64 -1, i64* [[PTR]], align 8
262+
; CHECK-NEXT: store volatile i64 -1, i64* [[PTR]], align 8
263+
; CHECK-NEXT: store volatile i64 -1, i64* [[PTR]], align 8
264+
; CHECK-NEXT: store volatile i64 -1, i64* [[PTR]], align 8
265+
; CHECK-NEXT: store volatile i64 -1, i64* [[PTR]], align 8
258266
; CHECK-NEXT: store volatile i64 2, i64* [[PTR]], align 8
259-
; CHECK-NEXT: br label [[COMMON_RET:%.*]]
260-
; CHECK: false2:
261-
; CHECK-NEXT: store volatile i64 3, i64* [[PTR]], align 8
262267
; CHECK-NEXT: br label [[COMMON_RET]]
263268
;
264269
br i1 %c, label %true1, label %false1

0 commit comments

Comments
 (0)