Skip to content

Commit 1badfbb

Browse files
committed
Fix incorrect TypeSize->uint64_t cast in InductionDescriptor::isInductionPHI
The code was relying upon the implicit conversion of TypeSize to uint64_t and assuming the type in question was always fixed. However, I discovered an issue when running the canon-freeze pass with some IR loops that contains scalable vector types. I've changed the code to bail out if the size is unknown at compile time, since we cannot compute whether the step is a multiple of the type size or not. I added a test here: Transforms/CanonicalizeFreezeInLoops/phis.ll Differential Revision: https://reviews.llvm.org/D118696
1 parent 09d2076 commit 1badfbb

File tree

2 files changed

+47
-2
lines changed

2 files changed

+47
-2
lines changed

llvm/lib/Analysis/IVDescriptors.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1428,10 +1428,14 @@ bool InductionDescriptor::isInductionPHI(
14281428

14291429
ConstantInt *CV = ConstStep->getValue();
14301430
const DataLayout &DL = Phi->getModule()->getDataLayout();
1431-
int64_t Size = static_cast<int64_t>(DL.getTypeAllocSize(ElementType));
1432-
if (!Size)
1431+
TypeSize TySize = DL.getTypeAllocSize(ElementType);
1432+
// TODO: We could potentially support this for scalable vectors if we can
1433+
// prove at compile time that the constant step is always a multiple of
1434+
// the scalable type.
1435+
if (TySize.isZero() || TySize.isScalable())
14331436
return false;
14341437

1438+
int64_t Size = static_cast<int64_t>(TySize.getFixedSize());
14351439
int64_t CVSize = CV->getSExtValue();
14361440
if (CVSize % Size)
14371441
return false;

llvm/unittests/Analysis/IVDescriptorsTest.cpp

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,47 @@ for.end:
9797
});
9898
}
9999

100+
TEST(IVDescriptorsTest, LoopWithScalableTypes) {
101+
// Parse the module.
102+
LLVMContext Context;
103+
104+
std::unique_ptr<Module> M =
105+
parseIR(Context,
106+
R"(define void @foo(<vscale x 4 x float>* %ptr) {
107+
entry:
108+
br label %for.body
109+
110+
for.body:
111+
%lsr.iv1 = phi <vscale x 4 x float>* [ %0, %for.body ], [ %ptr, %entry ]
112+
%j.0117 = phi i64 [ %inc, %for.body ], [ 0, %entry ]
113+
%lsr.iv12 = bitcast <vscale x 4 x float>* %lsr.iv1 to i8*
114+
%inc = add nuw nsw i64 %j.0117, 1
115+
%uglygep = getelementptr i8, i8* %lsr.iv12, i64 4
116+
%0 = bitcast i8* %uglygep to <vscale x 4 x float>*
117+
%cmp = icmp ne i64 %inc, 1024
118+
br i1 %cmp, label %for.body, label %end
119+
120+
end:
121+
ret void
122+
})");
123+
124+
runWithLoopInfoAndSE(
125+
*M, "foo", [&](Function &F, LoopInfo &LI, ScalarEvolution &SE) {
126+
Function::iterator FI = F.begin();
127+
// First basic block is entry - skip it.
128+
BasicBlock *Header = &*(++FI);
129+
assert(Header->getName() == "for.body");
130+
Loop *L = LI.getLoopFor(Header);
131+
EXPECT_NE(L, nullptr);
132+
PHINode *Inst_iv = dyn_cast<PHINode>(&Header->front());
133+
assert(Inst_iv->getName() == "lsr.iv1");
134+
InductionDescriptor IndDesc;
135+
bool IsInductionPHI =
136+
InductionDescriptor::isInductionPHI(Inst_iv, L, &SE, IndDesc);
137+
EXPECT_FALSE(IsInductionPHI);
138+
});
139+
}
140+
100141
// Depending on how SCEV deals with ptrtoint cast, the step of a phi could be
101142
// a pointer, and InductionDescriptor used to fail with an assertion.
102143
// So just check that it doesn't assert.

0 commit comments

Comments
 (0)