[PATCH] D122281: [AArch64][SelectionDAG] Refactor to support more scalable vector extending loads

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm accepted this revision.
paulwalker-arm added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:1250
+      setLoadExtAction(Op, MVT::nxv4i32, MVT::nxv4i8, Legal);
+      setLoadExtAction(Op, MVT::nxv2i32, MVT::nxv2i16, Legal);
+      setLoadExtAction(Op, MVT::nxv4i32, MVT::nxv4i16, Legal);
----------------
Allen wrote:
> Allen wrote:
> > paulwalker-arm wrote:
> > > `MVT::nxv2i32` is not a legal type and so shouldn't be listed here.
> > I add type MVT::nxv2i32 because the case masked_zload_2i16_2f64 in CodeGen/AArch64/sve-masked-ldst-zext.ll.
> > If I don't set it Legal, then there will be an redundant **and**, the complete assemble is followed.
> > ```
> >         ld1h	{ z0.d }, p0/z, [x0]
> > 	ptrue	p1.d
> > 	and	z0.d, z0.d, #0xffff   --- redundant ?
> > 	ucvtf	z0.d, p1/m, z0.d
> > ```
> > so both 64 and 128-bit vectors are supported extend ?
> hi, @paulwalker-arm: 
>    do you think we need not define MVT::nxv2i32 legal, and eliminate the **and** with other method ?
It does feel like we're missing an `ISD::AND` combine somewhere, but given you're just maintaining the existing behaviour I'll not worry about it for this patch.  We can circle back to the `MVT::nxv2i32` issue later.


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

https://reviews.llvm.org/D122281