[PATCH] D38677: [ConstantFold] Fix a crash when folding a GEP that has vector index
Matthew Simpson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 26 11:54:40 PDT 2017
mssimpso added a comment.
Haicheng,
How hard do you think it would be to update this transformation to reason about vector GEPs rather than bailing out? (I've left some inline comments if you want to give it a try). Having said that, I'm fine with that patch as is. But you should at least add a comment explaining why we're giving up.
================
Comment at: lib/IR/ConstantFold.cpp:2259
unsigned CommonExtendedWidth =
std::max(PrevIdx->getType()->getIntegerBitWidth(),
Div->getType()->getIntegerBitWidth());
----------------
haicheng wrote:
> The crash happens here. Type::getIntegerBitWidth() casts underlying type to IntegerType and crashes when the underlying type is a vector.
`getScalarScalarSizeInBits` should work for both scalars and vectors.
================
Comment at: lib/IR/ConstantFold.cpp:2265
// overflow trouble.
if (!PrevIdx->getType()->isIntegerTy(CommonExtendedWidth))
PrevIdx = ConstantExpr::getSExt(
----------------
haicheng wrote:
> I think this line also does not expect a vector PrevIdx.
`isIntOrIntVectorTy` should work for both scalars and vectors.
================
Comment at: lib/IR/ConstantFold.cpp:2272
NewIdxs[i - 1] = ConstantExpr::getAdd(PrevIdx, Div);
}
----------------
haicheng wrote:
> I think this line also does not expect a vector PrevIdx.
>
I believe the add should work for vectors as well, but if `PrevIdx` is a vector, I think you would have to broadcast `Div` to a vector.
Repository:
rL LLVM
https://reviews.llvm.org/D38677
More information about the llvm-commits
mailing list