Skip to content

Commit 748d583

Browse files
committed
Fix incorrect codegen with respect to GEPs llvm#85333
As mentioned in llvm#68882 and https://discourse.llvm.org/t/rfc-replacing-getelementptr-with-ptradd/68699 Gep arithmetic isn't consistent with different types. GVNSink didn't realize this and sank all geps as long as their operands can be wired via PHIs in a post-dominator. Fixes: llvm#85333
1 parent 1b942ae commit 748d583

File tree

2 files changed

+105
-5
lines changed

2 files changed

+105
-5
lines changed

llvm/lib/Transforms/Scalar/GVNSink.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -719,12 +719,11 @@ GVNSink::analyzeInstructionForSinking(LockstepReverseIterator &LRI,
719719
// try and continue making progress.
720720
Instruction *I0 = NewInsts[0];
721721

722-
// If all instructions that are going to participate don't have the same
723-
// number of operands, we can't do any useful PHI analysis for all operands.
724-
auto hasDifferentNumOperands = [&I0](Instruction *I) {
725-
return I->getNumOperands() != I0->getNumOperands();
722+
auto isNotSameOperation = [&I0](Instruction *I) {
723+
return !I0->isSameOperationAs(I);
726724
};
727-
if (any_of(NewInsts, hasDifferentNumOperands))
725+
726+
if (any_of(NewInsts, isNotSameOperation))
728727
return std::nullopt;
729728

730729
for (unsigned OpNum = 0, E = I0->getNumOperands(); OpNum != E; ++OpNum) {
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
2+
; RUN: opt -passes=gvn-sink -S %s | FileCheck %s
3+
4+
target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
5+
6+
%"struct.std::pair" = type <{ i32, %struct.substruct, [2 x i8] }>
7+
%struct.substruct = type { i8, i8 }
8+
%"struct.std::random_access_iterator_tag" = type { i8 }
9+
10+
; Check that gep is not sunk as they are of different types.
11+
define void @bar(ptr noundef nonnull align 4 dereferenceable(4) %__i, i32 noundef %__n) {
12+
; CHECK-LABEL: define void @bar(
13+
; CHECK-SAME: ptr noundef nonnull align 4 dereferenceable(4) [[__I:%.*]], i32 noundef [[__N:%.*]]) {
14+
; CHECK-NEXT: entry:
15+
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[__N]], 1
16+
; CHECK-NEXT: br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
17+
; CHECK: if.then:
18+
; CHECK-NEXT: [[TMP0:%.*]] = load ptr, ptr [[__I]], align 4
19+
; CHECK-NEXT: [[INCDEC_PTR4:%.*]] = getelementptr inbounds i8, ptr [[TMP0]], i32 -8
20+
; CHECK-NEXT: br label [[IF_END6:%.*]]
21+
; CHECK: if.else:
22+
; CHECK-NEXT: [[TMP1:%.*]] = load ptr, ptr [[__I]], align 4
23+
; CHECK-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds %"struct.std::pair", ptr [[TMP1]], i32 [[__N]]
24+
; CHECK-NEXT: br label [[IF_END6]]
25+
; CHECK: if.end6:
26+
; CHECK-NEXT: [[INCDEC_PTR_SINK:%.*]] = phi ptr [ [[INCDEC_PTR4]], [[IF_THEN]] ], [ [[ADD_PTR]], [[IF_ELSE]] ]
27+
; CHECK-NEXT: store ptr [[INCDEC_PTR_SINK]], ptr [[__I]], align 4
28+
; CHECK-NEXT: ret void
29+
;
30+
entry:
31+
%cmp = icmp eq i32 %__n, 1
32+
br i1 %cmp, label %if.then, label %if.else
33+
34+
if.then:
35+
%3 = load ptr, ptr %__i, align 4
36+
%incdec.ptr4 = getelementptr inbounds i8, ptr %3, i32 -8
37+
br label %if.end6
38+
39+
if.else:
40+
%4 = load ptr, ptr %__i, align 4
41+
%add.ptr = getelementptr inbounds %"struct.std::pair", ptr %4, i32 %__n
42+
br label %if.end6
43+
44+
if.end6:
45+
%incdec.ptr.sink = phi ptr [ %incdec.ptr4, %if.then ], [ %add.ptr, %if.else ]
46+
store ptr %incdec.ptr.sink, ptr %__i, align 4
47+
ret void
48+
}
49+
50+
; Check that load,gep, and store are all sunk as they are safe to do.
51+
define void @foo(ptr noundef nonnull align 4 dereferenceable(4) %__i, i32 noundef %__n) {
52+
; CHECK-LABEL: define void @foo(
53+
; CHECK-SAME: ptr noundef nonnull align 4 dereferenceable(4) [[__I:%.*]], i32 noundef [[__N:%.*]]) {
54+
; CHECK-NEXT: entry:
55+
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[__N]], 1
56+
; CHECK-NEXT: br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
57+
; CHECK: if.then:
58+
; CHECK-NEXT: br label [[IF_END6:%.*]]
59+
; CHECK: if.else:
60+
; CHECK-NEXT: [[CMP2:%.*]] = icmp eq i32 [[__N]], -1
61+
; CHECK-NEXT: br i1 [[CMP2]], label [[IF_THEN3:%.*]], label [[IF_ELSE5:%.*]]
62+
; CHECK: if.then3:
63+
; CHECK-NEXT: br label [[IF_END6]]
64+
; CHECK: if.else5:
65+
; CHECK-NEXT: br label [[IF_END6]]
66+
; CHECK: if.end6:
67+
; CHECK-NEXT: [[DOTSINK1:%.*]] = phi i32 [ 8, [[IF_THEN]] ], [ -8, [[IF_THEN3]] ], [ -4, [[IF_ELSE5]] ]
68+
; CHECK-NEXT: [[TMP0:%.*]] = load ptr, ptr [[__I]], align 4
69+
; CHECK-NEXT: [[INCDEC_PTR:%.*]] = getelementptr inbounds i8, ptr [[TMP0]], i32 [[DOTSINK1]]
70+
; CHECK-NEXT: store ptr [[INCDEC_PTR]], ptr [[__I]], align 4
71+
; CHECK-NEXT: ret void
72+
;
73+
entry:
74+
%cmp = icmp eq i32 %__n, 1
75+
br i1 %cmp, label %if.then, label %if.else
76+
77+
if.then:
78+
%1 = load ptr, ptr %__i, align 4
79+
%incdec.ptr = getelementptr inbounds i8, ptr %1, i32 8
80+
store ptr %incdec.ptr, ptr %__i, align 4
81+
br label %if.end6
82+
83+
if.else:
84+
%cmp2 = icmp eq i32 %__n, -1
85+
br i1 %cmp2, label %if.then3, label %if.else5
86+
87+
if.then3:
88+
%3 = load ptr, ptr %__i, align 4
89+
%incdec.ptr4 = getelementptr inbounds i8, ptr %3, i32 -8
90+
store ptr %incdec.ptr4, ptr %__i, align 4
91+
br label %if.end6
92+
93+
if.else5:
94+
%4 = load ptr, ptr %__i, align 4
95+
%add.ptr = getelementptr inbounds i8, ptr %4, i32 -4
96+
store ptr %add.ptr, ptr %__i, align 4
97+
br label %if.end6
98+
99+
if.end6:
100+
ret void
101+
}

0 commit comments

Comments
 (0)