Skip to content

Commit 7d0648c

Browse files
Alex Gateanikic
authored andcommitted
[GVN] Patch for invalid GVN replacement
If PRE is performed as part of the main GVN pass (to PRE GEP operands before processing loads), and it is performed across a backedge, we will end up adding the new instruction to the leader table of a block that has not yet been processed. When it will be processed, GVN will incorrectly assume that the value is already available, even though it is only available at the end of the block. Avoid this by not performing PRE across backedges. Fixes llvm#58418. Differential Revision: https://reviews.llvm.org/D136095
1 parent 1fb1861 commit 7d0648c

File tree

3 files changed

+50
-13
lines changed

3 files changed

+50
-13
lines changed

llvm/lib/Transforms/Scalar/GVN.cpp

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2828,17 +2828,10 @@ bool GVNPass::performScalarPRE(Instruction *CurInst) {
28282828
NumWithout = 2;
28292829
break;
28302830
}
2831-
// It is not safe to do PRE when P->CurrentBlock is a loop backedge, and
2832-
// when CurInst has operand defined in CurrentBlock (so it may be defined
2833-
// by phi in the loop header).
2831+
// It is not safe to do PRE when P->CurrentBlock is a loop backedge.
28342832
assert(BlockRPONumber.count(P) && BlockRPONumber.count(CurrentBlock) &&
28352833
"Invalid BlockRPONumber map.");
2836-
if (BlockRPONumber[P] >= BlockRPONumber[CurrentBlock] &&
2837-
llvm::any_of(CurInst->operands(), [&](const Use &U) {
2838-
if (auto *Inst = dyn_cast<Instruction>(U.get()))
2839-
return Inst->getParent() == CurrentBlock;
2840-
return false;
2841-
})) {
2834+
if (BlockRPONumber[P] >= BlockRPONumber[CurrentBlock]) {
28422835
NumWithout = 2;
28432836
break;
28442837
}
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 -gvn -S < %s | FileCheck %s
3+
4+
; Check that PRE-LOAD across backedge does not
5+
; result in invalid dominator tree.
6+
declare void @use(i32)
7+
8+
define void @test1(i1 %c, i32 %arg) {
9+
; CHECK-LABEL: @test1(
10+
; CHECK-NEXT: br i1 [[C:%.*]], label [[BB1:%.*]], label [[DOTBB2_CRIT_EDGE:%.*]]
11+
; CHECK: .bb2_crit_edge:
12+
; CHECK-NEXT: [[DOTPRE:%.*]] = shl i32 [[ARG:%.*]], 2
13+
; CHECK-NEXT: br label [[BB2:%.*]]
14+
; CHECK: bb1:
15+
; CHECK-NEXT: [[SHL1:%.*]] = shl i32 [[ARG]], 2
16+
; CHECK-NEXT: br label [[BB3:%.*]]
17+
; CHECK: bb2:
18+
; CHECK-NEXT: [[SHL2_PRE_PHI:%.*]] = phi i32 [ [[DOTPRE]], [[DOTBB2_CRIT_EDGE]] ], [ [[SHL3:%.*]], [[BB3]] ]
19+
; CHECK-NEXT: call void @use(i32 [[SHL2_PRE_PHI]])
20+
; CHECK-NEXT: br label [[BB3]]
21+
; CHECK: bb3:
22+
; CHECK-NEXT: [[SHL3]] = shl i32 [[ARG]], 2
23+
; CHECK-NEXT: [[GEP:%.*]] = getelementptr i32, ptr null, i32 [[SHL3]]
24+
; CHECK-NEXT: [[V:%.*]] = load i32, ptr [[GEP]], align 4
25+
; CHECK-NEXT: call void @use(i32 [[V]])
26+
; CHECK-NEXT: br label [[BB2]]
27+
;
28+
br i1 %c, label %bb1, label %bb2
29+
30+
bb1:
31+
%shl1 = shl i32 %arg, 2
32+
br label %bb3
33+
34+
bb2:
35+
%shl2 = shl i32 %arg, 2
36+
call void @use(i32 %shl2)
37+
br label %bb3
38+
39+
bb3:
40+
%shl3 = shl i32 %arg, 2
41+
%gep = getelementptr i32, ptr null, i32 %shl3
42+
%v = load i32, ptr %gep, align 4
43+
call void @use(i32 %v)
44+
br label %bb2
45+
}

llvm/test/Transforms/GVN/PRE/pre-gep-load.ll

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,14 +95,13 @@ define void @test_shortcut_safe(i1 %tst, i32 %p1, i32* %a) {
9595
; CHECK-LABEL: @test_shortcut_safe(
9696
; CHECK-NEXT: br i1 [[TST:%.*]], label [[SEXT1:%.*]], label [[PRE_DEST:%.*]]
9797
; CHECK: pre.dest:
98-
; CHECK-NEXT: [[DOTPRE:%.*]] = sext i32 [[P1:%.*]] to i64
9998
; CHECK-NEXT: br label [[SEXT_USE:%.*]]
10099
; CHECK: sext1:
101-
; CHECK-NEXT: [[IDXPROM:%.*]] = sext i32 [[P1]] to i64
100+
; CHECK-NEXT: [[IDXPROM:%.*]] = sext i32 [[P1:%.*]] to i64
102101
; CHECK-NEXT: br label [[SEXT_USE]]
103102
; CHECK: sext.use:
104-
; CHECK-NEXT: [[IDXPROM2_PRE_PHI:%.*]] = phi i64 [ [[IDXPROM]], [[SEXT1]] ], [ [[DOTPRE]], [[PRE_DEST]] ]
105-
; CHECK-NEXT: [[ARRAYIDX3:%.*]] = getelementptr inbounds i32, i32* [[A:%.*]], i64 [[IDXPROM2_PRE_PHI]]
103+
; CHECK-NEXT: [[IDXPROM2:%.*]] = sext i32 [[P1]] to i64
104+
; CHECK-NEXT: [[ARRAYIDX3:%.*]] = getelementptr inbounds i32, i32* [[A:%.*]], i64 [[IDXPROM2]]
106105
; CHECK-NEXT: [[VAL:%.*]] = load i32, i32* [[ARRAYIDX3]], align 4
107106
; CHECK-NEXT: tail call void @g(i32 [[VAL]])
108107
; CHECK-NEXT: br label [[PRE_DEST]]

0 commit comments

Comments
 (0)