Skip to content

Commit a0fe6e1

Browse files
author
Vasileios Porpodas
committed
[SandboxVec][BottomUpVec] Fix packing when PHIs are present
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 c7053ac commit a0fe6e1

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)