[PATCH] D68337: [ARM][MVE] Enable extending masked loads

Sam Parker via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 7 08:55:39 PDT 2019


samparker marked an inline comment as done.
samparker added inline comments.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:8887
   // zero too, and other values are lowered to a select.
   SDValue ZeroVec = DAG.getNode(ARMISD::VMOVIMM, dl, VT,
                                 DAG.getTargetConstant(0, dl, MVT::i32));
----------------
samparker wrote:
> dmgreen wrote:
> > samparker wrote:
> > > dmgreen wrote:
> > > > This is creating a zero vector of size VT, which is the size of what the masked loads returns. Should it instead be the size of the memory being loaded (because the extend happens to the passthru as well)? What happens if that isn't a legal value type?
> > > Well, surely the result VT of the masked load has to match the VT of the passthru input. passthru is not about what memory is accessed, but what is written to the destination register. VOVIMM will also generate the same zero value for all full width vector types so for vector widths less than 128-bits, the higher elements will be zeroed and that makes sense. For vectors wider than 128-bits, I think something would have gone before here. I'll add some tests for both these cases.
> > Hmmm. Yeah OK. I see. The PassThru is explicitly extended in tryToFoldExtOfMaskedLoad?
> > 
> > That makes sense, and the tests look OK. (There's one that is both sext and zext the same value, but that looks correct for where it is used).
> > 
> > Test for masked loads/stores longer than 128 bits sounds like a good idea. We should ideally be able to deal with longer vector by splitting them just fine.
> At some point, I was extending passthru... but it seems that is no longer the case! Our VMOVIMM is probably keeping us correct and if I extend it in dag combine, hopefully we won't need the bitcast handling here anymore.
Ah, no. I was just being blind, passthru is extended.


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

https://reviews.llvm.org/D68337





More information about the llvm-commits mailing list