Skip to content

Commit ec0f678

Browse files
aleks-tmbAleksander Popov
andauthored
[GuardWidening] Fix widening possibility check (#66064)
In the 0e0ff85 was introduced inconsistency between condition widening and checking if it's possible to widen. We check the possibility to hoist checks parsed from the condition, but hoist entire condition. This patch returns testing that a condition can be hoisted rather than the checks parsed from that condition. Co-authored-by: Aleksander Popov <[email protected]>
1 parent 0528dbf commit ec0f678

File tree

2 files changed

+54
-5
lines changed

2 files changed

+54
-5
lines changed

llvm/lib/Transforms/Scalar/GuardWidening.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,7 @@ class GuardWideningImpl {
184184
/// Compute the score for widening the condition in \p DominatedInstr
185185
/// into \p WideningPoint.
186186
WideningScore computeWideningScore(Instruction *DominatedInstr,
187+
Instruction *ToWiden,
187188
Instruction *WideningPoint,
188189
SmallVectorImpl<Value *> &ChecksToHoist,
189190
SmallVectorImpl<Value *> &ChecksToWiden);
@@ -423,8 +424,8 @@ bool GuardWideningImpl::eliminateInstrViaWidening(
423424
continue;
424425
SmallVector<Value *> CandidateChecks;
425426
parseWidenableGuard(Candidate, CandidateChecks);
426-
auto Score = computeWideningScore(Instr, WideningPoint, ChecksToHoist,
427-
CandidateChecks);
427+
auto Score = computeWideningScore(Instr, Candidate, WideningPoint,
428+
ChecksToHoist, CandidateChecks);
428429
LLVM_DEBUG(dbgs() << "Score between " << *Instr << " and " << *Candidate
429430
<< " is " << scoreTypeToString(Score) << "\n");
430431
if (Score > BestScoreSoFar) {
@@ -456,8 +457,8 @@ bool GuardWideningImpl::eliminateInstrViaWidening(
456457
}
457458

458459
GuardWideningImpl::WideningScore GuardWideningImpl::computeWideningScore(
459-
Instruction *DominatedInstr, Instruction *WideningPoint,
460-
SmallVectorImpl<Value *> &ChecksToHoist,
460+
Instruction *DominatedInstr, Instruction *ToWiden,
461+
Instruction *WideningPoint, SmallVectorImpl<Value *> &ChecksToHoist,
461462
SmallVectorImpl<Value *> &ChecksToWiden) {
462463
Loop *DominatedInstrLoop = LI.getLoopFor(DominatedInstr->getParent());
463464
Loop *DominatingGuardLoop = LI.getLoopFor(WideningPoint->getParent());
@@ -475,7 +476,10 @@ GuardWideningImpl::WideningScore GuardWideningImpl::computeWideningScore(
475476

476477
if (!canBeHoistedTo(ChecksToHoist, WideningPoint))
477478
return WS_IllegalOrNegative;
478-
if (!canBeHoistedTo(ChecksToWiden, WideningPoint))
479+
// Further in the GuardWideningImpl::hoistChecks the entire condition might be
480+
// widened, not the parsed list of checks. So we need to check the possibility
481+
// of that condition hoisting.
482+
if (!canBeHoistedTo(getCondition(ToWiden), WideningPoint))
479483
return WS_IllegalOrNegative;
480484

481485
// If the guard was conditional executed, it may never be reached
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
2+
; RUN: opt -S -passes=guard-widening < %s | FileCheck %s
3+
4+
declare void @llvm.experimental.deoptimize.isVoid(...)
5+
declare i1 @llvm.experimental.widenable.condition()
6+
7+
; In the current scheme we widen `br i1 %and1`, so we try to build
8+
; `and i1 poison, %and1` using `%call0` as insertion point.
9+
; That's not possible, so widening will not occur in the case.
10+
; TODO: Widen `%call0` in the test case, not a branch.
11+
define void @test() {
12+
; CHECK-LABEL: @test(
13+
; CHECK-NEXT: bb0:
14+
; CHECK-NEXT: [[CALL0:%.*]] = call i1 @llvm.experimental.widenable.condition()
15+
; CHECK-NEXT: [[AND0:%.*]] = and i1 false, [[CALL0]]
16+
; CHECK-NEXT: [[AND1:%.*]] = and i1 false, [[AND0]]
17+
; CHECK-NEXT: br i1 [[AND1]], label [[BB1:%.*]], label [[DEOPT:%.*]]
18+
; CHECK: bb1:
19+
; CHECK-NEXT: [[CALL1:%.*]] = call i1 @llvm.experimental.widenable.condition()
20+
; CHECK-NEXT: [[AND2:%.*]] = and i1 poison, [[CALL1]]
21+
; CHECK-NEXT: br i1 [[AND2]], label [[UNREACH:%.*]], label [[DEOPT]]
22+
; CHECK: unreach:
23+
; CHECK-NEXT: unreachable
24+
; CHECK: deopt:
25+
; CHECK-NEXT: call void (...) @llvm.experimental.deoptimize.isVoid(i32 0) [ "deopt"(i32 0) ]
26+
; CHECK-NEXT: ret void
27+
;
28+
bb0:
29+
%call0 = call i1 @llvm.experimental.widenable.condition()
30+
%and0 = and i1 false, %call0
31+
%and1 = and i1 false, %and0
32+
br i1 %and1, label %bb1, label %deopt
33+
34+
bb1:
35+
%call1 = call i1 @llvm.experimental.widenable.condition()
36+
%and2 = and i1 poison, %call1
37+
br i1 %and2, label %unreach, label %deopt
38+
39+
unreach:
40+
unreachable
41+
42+
deopt:
43+
call void (...) @llvm.experimental.deoptimize.isVoid(i32 0) [ "deopt"(i32 0) ]
44+
ret void
45+
}

0 commit comments

Comments
 (0)