[PATCH] D75064: [AArch64][SVE] Add support for DestructiveBinaryImm DestructiveInstType

[Sander de Smalen via Phabricator via llvm-commits]

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

LGTM!



================
Comment at: llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td:1107
 
-  defm ASR_ZPmZ  : sve_int_bin_pred_shift<0b000, "asr", int_aarch64_sve_asr>;
-  defm LSR_ZPmZ  : sve_int_bin_pred_shift<0b001, "lsr", int_aarch64_sve_lsr>;
-  defm LSL_ZPmZ  : sve_int_bin_pred_shift<0b011, "lsl", int_aarch64_sve_lsl>;
-  defm ASRR_ZPmZ : sve_int_bin_pred_shift<0b100, "asrr", null_frag>;
-  defm LSRR_ZPmZ : sve_int_bin_pred_shift<0b101, "lsrr", null_frag>;
-  defm LSLR_ZPmZ : sve_int_bin_pred_shift<0b111, "lslr", null_frag>;
+  defm ASR_ZPZZ  : sve_int_bin_pred_zx<int_aarch64_sve_asr>;
+  defm LSR_ZPZZ  : sve_int_bin_pred_zx<int_aarch64_sve_lsr>;
----------------
cameron.mcinally wrote:
> sdesmalen wrote:
> > The _zx suggests it covers the merging with undef case as well. Are those patterns you plan to add in this patch?
> I could add those if you'd like. They're there in D71712.
> 
> I'm trying to keep the size of the patches to a minimum for reviewing purposes, so that's why these patches might seem short.
> 
> My general plan is to get all the framework (e.g. AArch64ExpandPseudoInsts.cpp changes) in first. That way adding patterns and tests later should be very easy to review. If you'd like me to upstream this in another way, I'm open to suggestions...
Okay that's fine with me, it was more that I was wondering whether we'd keep _zx in that case (as opposed to _z), but I'm happy to keep it as is for now.


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

https://reviews.llvm.org/D75064