[PATCH] D72451: [ARM][MVE] Enable extending gathers
Dave Green via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 9 12:35:28 PST 2020
dmgreen added inline comments.
================
Comment at: llvm/lib/Target/ARM/MVEGatherScatterLowering.cpp:140
+ if (OriginalTy->getVectorNumElements() == 4)
+ ResultTy = VectorType::get(OriginalTy->getInt32Ty(C), 4);
+ else if (OriginalTy->getVectorNumElements() == 8)
----------------
Should this be Type::getInt32Ty ? Or Builder.getInt32() ?
================
Comment at: llvm/lib/Target/ARM/MVEGatherScatterLowering.cpp:146
+ Instruction *Parent = nullptr;
+ for (User *u : I->users()) {
+ // Only do this to gathers with exactly one use
----------------
u -> U. Or better yet just check that I->hasOneUse(). The Use of a Instruction will always be an Instruction AFAIU.
Also "Parent" makes me think of the BasicBlock/Function, not the user. Maybe just call it "User" or something like it?
================
Comment at: llvm/lib/Target/ARM/MVEGatherScatterLowering.cpp:230
int GEPElemSize = GEP->getResultElementType()->getPrimitiveSizeInBits();
- int ResultElemSize = Ty->getScalarSizeInBits();
+ int ResultElemSize = ResultTy->getScalarSizeInBits();
// This can be a 32bit load scaled by 4, a 16bit load scaled by 2, or a
----------------
There are some tests in mve-gather-ind32-scaled.ll like "zext_signed_scaled_i16_i16" that I think can be converted to a `VLDRH.u32 Qd, [base, offs.sext, uxtw #1]` if we can get the extend and this scale working together. It's the type of the memory being loaded that's important for the scale, not the type of the result. (Err, I think. Check that sounds right)
================
Comment at: llvm/lib/Target/ARM/MVEGatherScatterLowering.cpp:268
+ // If this was an extending gather, also get rid of the gather instruction
+ // itself, to avoid reevaluation
+ I->eraseFromParent();
----------------
Maybe drop the "to avoid reevaluation". The rest is reason enough to remove the gather.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72451/new/
https://reviews.llvm.org/D72451
More information about the llvm-commits
mailing list