[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