[PATCH] D70176: [Codegen][ARM] Add addressing modes from masked loads and stores

[Sam Parker via Phabricator via llvm-commits]

samparker accepted this revision.
samparker added a comment.
This revision is now accepted and ready to land.

Cheers. LGTM



================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:387
+      setIndexedMaskedLoadAction(im, VT, Legal);
+      setIndexedMaskedStoreAction(im, VT, Legal);
+    }
----------------
dmgreen wrote:
> samparker wrote:
> > Why not v4i32 and floats too?
> This is "extending masked post-inc stores", so is only the extended types that will be extended. The others are above.
> 
> We might well want to "zero extend" fp16s into a wider register at some point, especially if we are converting them to floats, but thats not a job for here.
Face palm. Yeah ok, we can cross fp16 if/when we need it.


================
Comment at: llvm/lib/Target/ARM/ARMInstrMVE.td:5312
+                                          PatFrag StoreKind, int shift>
+  : Pat<(StoreKind (Ty MQPR:$Rt), tGPR:$Rn, t2am_imm7_offset<shift>:$addr, VCCR:$pred),
+        (Opcode MQPR:$Rt, tGPR:$Rn, t2am_imm7_offset<shift>:$addr, (i32 1), VCCR:$pred)>;
----------------
dmgreen wrote:
> samparker wrote:
> > I don't think we shouldn't be restricting the base to a T1 register.
> Ooof, the double negatives! This uses the same as the MVE_vector_offset_store_typed, which I think is OK for "normal" loads/stores. It's the extending loads/stores below that might be the problem (and don't really look right to me).
> 
> I'll make it the same a non-masked for the moment, and try to fixup what doesn't look right in another commit.
Haha, my bad. Okay.


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

https://reviews.llvm.org/D70176