[PATCH] D95456: [ARM] i16 insert-of-extract to VINS pattern

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMInstrMVE.td:1882
+  def : Pat<(ARMinsertelt (v8i16 MQPR:$src1), (ARMvgetlaneu (v8i16 MQPR:$src2), imm_even:$extlane), imm_odd:$inslane),
+            (COPY_TO_REGCLASS (INSERT_SUBREG (v4f32 (COPY_TO_REGCLASS MQPR:$src1, MQPR)),
+                                (VINSH (EXTRACT_SUBREG MQPR:$src1, (SSubReg_f16_reg imm_odd:$inslane)),
----------------
simon_tatham wrote:
> dmgreen wrote:
> > simon_tatham wrote:
> > > I don't understand this part – if $src1 is already an MQPR, why does it need a COPY_TO_REGCLASS?
> > Hmm. I added it as a typecast, essentially. Otherwise the INSERT_SUBREG fails to make it through the tablegen type checks. Trying it as `(INSERT_SUBREG (v4f32 MQPR:$src1), ...` gives an error that looks like the insertsubreg has conflicting input types
> > `(INSERT_SUBREG:{ *:[v4f32] } MQPR:{ *:[] }:$src1`
> Ah, I see – it's not that you needed to convert from MQPR to MQPR, it's just that a side effect of that is that you also get to convert from v8i16 to v4f32, which was what you really needed.
> 
> In that case, is that implicit conversion from v8i16 to v4f32 acting as a bitcast, or a VECTOR_REG_CAST? Does this pattern do the right thing when tested big-endian?
The COPY machine instruction (which is what a COPY_TO_REGCLASS will eventually turn into) loses any type info so will always act as a VECTOR_REG_CAST. So should be fine in big and little endian - and seems to be OK in my testing.


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

https://reviews.llvm.org/D95456