Skip to content

Commit d2234ca

Browse files
author
vporpo
authored
[SandboxVec][BottomUpVec] Fix packing when PHIs are present (llvm#124206)
Before this patch we might have emitted pack instructions in between PHI nodes. This patch fixes it by fixing the insert point of the new packs.
1 parent 33c4407 commit d2234ca

File tree

5 files changed

+151
-4
lines changed

5 files changed

+151
-4
lines changed

‎llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/VecUtils.h‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,19 @@ class VecUtils {
137137
}
138138
return LowestI;
139139
}
140+
141+
/// If \p I is not a PHI it returns it. Else it walks down the instruction
142+
/// chain looking for the last PHI and returns it. \Returns nullptr if \p I is
143+
/// nullptr.
144+
static Instruction *getLastPHIOrSelf(Instruction *I) {
145+
Instruction *LastI = I;
146+
while (I != nullptr && isa<PHINode>(I)) {
147+
LastI = I;
148+
I = I->getNextNode();
149+
}
150+
return LastI;
151+
}
152+
140153
/// If all values in \p Bndl are of the same scalar type then return it,
141154
/// otherwise return nullptr.
142155
static Type *tryGetCommonScalarType(ArrayRef<Value *> Bndl) {

‎llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,13 @@ static SmallVector<Value *, 4> getOperand(ArrayRef<Value *> Bndl,
4747
/// of BB if no instruction found in \p Vals.
4848
static BasicBlock::iterator getInsertPointAfterInstrs(ArrayRef<Value *> Vals,
4949
BasicBlock *BB) {
50-
auto *BotI = VecUtils::getLowest(Vals);
50+
auto *BotI = VecUtils::getLastPHIOrSelf(VecUtils::getLowest(Vals));
5151
if (BotI == nullptr)
52-
// We are using BB->begin() as the fallback insert point if `ToPack` did
53-
// not contain instructions.
54-
return BB->begin();
52+
// We are using BB->begin() (or after PHIs) as the fallback insert point.
53+
return BB->empty()
54+
? BB->begin()
55+
: std::next(
56+
VecUtils::getLastPHIOrSelf(&*BB->begin())->getIterator());
5557
return std::next(BotI->getIterator());
5658
}
5759

‎llvm/test/Transforms/SandboxVectorizer/bottomup_basic.ll‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,3 +269,31 @@ define void @diamondMultiInput(ptr %ptr, ptr %ptrX) {
269269
store float %sub1, ptr %ptr1
270270
ret void
271271
}
272+
273+
define void @diamondWithConstantVector(ptr %ptr) {
274+
; CHECK-LABEL: define void @diamondWithConstantVector(
275+
; CHECK-SAME: ptr [[PTR:%.*]]) {
276+
; CHECK-NEXT: [[GEPA0:%.*]] = getelementptr i32, ptr [[PTR]], i64 0
277+
; CHECK-NEXT: [[GEPB0:%.*]] = getelementptr i32, ptr [[PTR]], i64 10
278+
; CHECK-NEXT: store <2 x i32> zeroinitializer, ptr [[GEPA0]], align 4
279+
; CHECK-NEXT: store <2 x i32> zeroinitializer, ptr [[GEPB0]], align 4
280+
; CHECK-NEXT: ret void
281+
;
282+
%gepA0 = getelementptr i32, ptr %ptr, i64 0
283+
%gepA1 = getelementptr i32, ptr %ptr, i64 1
284+
285+
%gepB0 = getelementptr i32, ptr %ptr, i64 10
286+
%gepB1 = getelementptr i32, ptr %ptr, i64 11
287+
288+
%zext0 = zext i16 0 to i32
289+
%zext1 = zext i16 0 to i32
290+
291+
store i32 %zext0, ptr %gepA0
292+
store i32 %zext1, ptr %gepA1
293+
294+
%orB0 = or i32 0, %zext0
295+
%orB1 = or i32 0, %zext1
296+
store i32 %orB0, ptr %gepB0
297+
store i32 %orB1, ptr %gepB1
298+
ret void
299+
}

‎llvm/test/Transforms/SandboxVectorizer/pack.ll‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,77 @@ define void @pack_constants(ptr %ptr) {
1414
store i8 1, ptr %ptr1
1515
ret void
1616
}
17+
18+
; Make sure we don't generate bad IR when packing PHIs.
19+
; NOTE: This test may become obsolete once we start vectorizing PHIs.
20+
define void @packPHIs(ptr %ptr) {
21+
; CHECK-LABEL: define void @packPHIs(
22+
; CHECK-SAME: ptr [[PTR:%.*]]) {
23+
; CHECK-NEXT: [[ENTRY:.*]]:
24+
; CHECK-NEXT: br label %[[LOOP:.*]]
25+
; CHECK: [[LOOP]]:
26+
; CHECK-NEXT: [[PHI0:%.*]] = phi i8 [ 0, %[[ENTRY]] ], [ 1, %[[LOOP]] ]
27+
; CHECK-NEXT: [[PHI1:%.*]] = phi i8 [ 0, %[[ENTRY]] ], [ 1, %[[LOOP]] ]
28+
; CHECK-NEXT: [[PHI2:%.*]] = phi i8 [ 0, %[[ENTRY]] ], [ 1, %[[LOOP]] ]
29+
; CHECK-NEXT: [[PHI3:%.*]] = phi i8 [ 0, %[[ENTRY]] ], [ 1, %[[LOOP]] ]
30+
; CHECK-NEXT: [[PACK:%.*]] = insertelement <2 x i8> poison, i8 [[PHI0]], i32 0
31+
; CHECK-NEXT: [[PACK1:%.*]] = insertelement <2 x i8> [[PACK]], i8 [[PHI1]], i32 1
32+
; CHECK-NEXT: [[GEP0:%.*]] = getelementptr i8, ptr [[PTR]], i64 0
33+
; CHECK-NEXT: store <2 x i8> [[PACK1]], ptr [[GEP0]], align 1
34+
; CHECK-NEXT: br label %[[LOOP]]
35+
; CHECK: [[EXIT:.*:]]
36+
; CHECK-NEXT: ret void
37+
;
38+
entry:
39+
br label %loop
40+
41+
loop:
42+
%phi0 = phi i8 [0, %entry], [1, %loop]
43+
%phi1 = phi i8 [0, %entry], [1, %loop]
44+
%phi2 = phi i8 [0, %entry], [1, %loop]
45+
%phi3 = phi i8 [0, %entry], [1, %loop]
46+
%gep0 = getelementptr i8, ptr %ptr, i64 0
47+
%gep1 = getelementptr i8, ptr %ptr, i64 1
48+
store i8 %phi0, ptr %gep0
49+
store i8 %phi1, ptr %gep1
50+
br label %loop
51+
52+
exit:
53+
ret void
54+
}
55+
56+
define void @packFromOtherBB(ptr %ptr, i8 %val) {
57+
; CHECK-LABEL: define void @packFromOtherBB(
58+
; CHECK-SAME: ptr [[PTR:%.*]], i8 [[VAL:%.*]]) {
59+
; CHECK-NEXT: [[ENTRY:.*]]:
60+
; CHECK-NEXT: [[ADD0:%.*]] = add i8 [[VAL]], 0
61+
; CHECK-NEXT: [[MUL1:%.*]] = mul i8 [[VAL]], 1
62+
; CHECK-NEXT: [[PACK:%.*]] = insertelement <2 x i8> poison, i8 [[ADD0]], i32 0
63+
; CHECK-NEXT: [[PACK1:%.*]] = insertelement <2 x i8> [[PACK]], i8 [[MUL1]], i32 1
64+
; CHECK-NEXT: br label %[[LOOP:.*]]
65+
; CHECK: [[LOOP]]:
66+
; CHECK-NEXT: [[PHI0:%.*]] = phi i8 [ 0, %[[ENTRY]] ], [ 1, %[[LOOP]] ]
67+
; CHECK-NEXT: [[PHI1:%.*]] = phi i8 [ 0, %[[ENTRY]] ], [ 1, %[[LOOP]] ]
68+
; CHECK-NEXT: [[GEP0:%.*]] = getelementptr i8, ptr [[PTR]], i64 0
69+
; CHECK-NEXT: store <2 x i8> [[PACK1]], ptr [[GEP0]], align 1
70+
; CHECK-NEXT: br label %[[LOOP]]
71+
; CHECK: [[EXIT:.*:]]
72+
; CHECK-NEXT: ret void
73+
;
74+
entry:
75+
%add0 = add i8 %val, 0
76+
%mul1 = mul i8 %val, 1
77+
br label %loop
78+
79+
loop:
80+
%phi0 = phi i8 [0, %entry], [1, %loop]
81+
%phi1 = phi i8 [0, %entry], [1, %loop]
82+
%gep0 = getelementptr i8, ptr %ptr, i64 0
83+
%gep1 = getelementptr i8, ptr %ptr, i64 1
84+
store i8 %add0, ptr %gep0
85+
store i8 %mul1, ptr %gep1
86+
br label %loop
87+
88+
exit:
89+
ret void
90+
}

‎llvm/unittests/Transforms/Vectorize/SandboxVectorizer/VecUtilsTest.cpp‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -481,6 +481,36 @@ define void @foo(i8 %v) {
481481
EXPECT_EQ(sandboxir::VecUtils::getLowest(DiffBBs), nullptr);
482482
}
483483

484+
TEST_F(VecUtilsTest, GetLastPHIOrSelf) {
485+
parseIR(R"IR(
486+
define void @foo(i8 %v) {
487+
entry:
488+
br label %bb1
489+
490+
bb1:
491+
%phi1 = phi i8 [0, %entry], [1, %bb1]
492+
%phi2 = phi i8 [0, %entry], [1, %bb1]
493+
br label %bb1
494+
495+
bb2:
496+
ret void
497+
}
498+
)IR");
499+
Function &LLVMF = *M->getFunction("foo");
500+
501+
sandboxir::Context Ctx(C);
502+
auto &F = *Ctx.createFunction(&LLVMF);
503+
auto &BB = getBasicBlockByName(F, "bb1");
504+
auto It = BB.begin();
505+
auto *PHI1 = cast<sandboxir::PHINode>(&*It++);
506+
auto *PHI2 = cast<sandboxir::PHINode>(&*It++);
507+
auto *Br = cast<sandboxir::BranchInst>(&*It++);
508+
EXPECT_EQ(sandboxir::VecUtils::getLastPHIOrSelf(PHI1), PHI2);
509+
EXPECT_EQ(sandboxir::VecUtils::getLastPHIOrSelf(PHI2), PHI2);
510+
EXPECT_EQ(sandboxir::VecUtils::getLastPHIOrSelf(Br), Br);
511+
EXPECT_EQ(sandboxir::VecUtils::getLastPHIOrSelf(nullptr), nullptr);
512+
}
513+
484514
TEST_F(VecUtilsTest, GetCommonScalarType) {
485515
parseIR(R"IR(
486516
define void @foo(i8 %v, ptr %ptr) {

0 commit comments

Comments
 (0)