[PATCH] D89872: [SVE][AArch64] Fix TypeSize warning in GEP cost analysis

Sander de Smalen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 23 11:09:21 PDT 2020


sdesmalen added inline comments.


================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfoImpl.h:785
     bool HasBaseReg = (BaseGV == nullptr);
 
     auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());
----------------
fpetrogalli wrote:
> sdesmalen wrote:
> > If any of the indices is a scalable vector, then the result type will also be a scalable vector. So you can bail out at the start of the function.
> My suggestion was to bail out in the place where the different types where handled, and where computation of the offset was done (the `else` after `if (StructType)`), so that we would have known where to start modifying the code for making `BaseOffset`  able to support scalable vectors. I have accepted the patch, but feel free to follow Sander's suggestion if you prefer this early exit (which was your original code too IIRC!).
> 
> 
Okay, you can ignore my previous comment as I got confused between the indexed type being a scalable vector, and the index itself being a scalable vector. You'll indeed want to add the check in the loop itself, as the element type of a `<vscale x 4 x i32>*` is the the indexed type `<vscale x 4 x i32>`, for which the size is being queried. The code in the previous revision of the patch (https://reviews.llvm.org/D89872?id=299716) was correct.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89872/new/

https://reviews.llvm.org/D89872



More information about the llvm-commits mailing list