[PATCH] D149369: [RISCV] Consolidate legality checking for strided load/store [nfc]
Craig Topper via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 28 09:43:49 PDT 2023
craig.topper added inline comments.
================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:11255
// Get the widened scalar type, e.g. v4i8 -> i64
unsigned WideScalarBitWidth =
BaseLdVT.getScalarSizeInBits() * BaseLdVT.getVectorNumElements();
----------------
reames wrote:
> craig.topper wrote:
> > reames wrote:
> > > luke wrote:
> > > > craig.topper wrote:
> > > > > Is WideScalarBitWidth guaranteed to be a valid MVT? Like can we get i256, i512, i1024, etc. here?
> > > > I presume if it's not then WideVecVT won't be legal below and we'll fail the legality check
> > > This was my reasoning as well.
> > My concern is that MVT::getIntegerVT might assert first.
> Just to be clear, you're talking about a possible bug in the code before my change right? I don't see anything in this diff which sounds relevant to your last clarification.
>
> If so, yes, I agree that looks suspicious.
Yes it would be a bug before your change. Your patch just made me focus more on the type checking than I did in the original review.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D149369/new/
https://reviews.llvm.org/D149369
More information about the llvm-commits
mailing list