[PATCH] D131875: [AArch64][SVE] Add hadd and rhadd support

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:5146-5160
+  case Intrinsic::aarch64_sve_srhadd:
+  case Intrinsic::aarch64_sve_urhadd:
+  case Intrinsic::aarch64_sve_shadd:
+  case Intrinsic::aarch64_sve_uhadd: {
+    bool IsSignedAdd = (IntNo == Intrinsic::aarch64_sve_srhadd ||
+                        IntNo == Intrinsic::aarch64_sve_shadd);
+    bool IsRoundingAdd = (IntNo == Intrinsic::aarch64_sve_srhadd ||
----------------
We use the `_PRED` suffix for nodes where the result of inactive lanes does not matter.  This means this transform is not strictly safe (as in somebody might do something erroneous based on the name).

We have `convertMergedOpToPredOp` to convert from predicated intrinsics to `_PRED` nodes when it is safe to do so. The alternative is to change the ISD name but that seems wrong because for the IR side of things you really do want `_PRED` nodes to free up isel/register allocation by selecting.


================
Comment at: llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td:3295
   // SVE2 integer halving add/subtract (predicated)
-  defm SHADD_ZPmZ  : sve2_int_arith_pred<0b100000, "shadd",  int_aarch64_sve_shadd>;
-  defm UHADD_ZPmZ  : sve2_int_arith_pred<0b100010, "uhadd",  int_aarch64_sve_uhadd>;
+  defm SHADD_ZPmZ  : sve2_int_arith_pred<0b100000, "shadd",  AArch64shadd_p>;
+  defm UHADD_ZPmZ  : sve2_int_arith_pred<0b100010, "uhadd",  AArch64uhadd_p>;
----------------
This ties in with my previous comment because typically here we'd create `SHADD_ZPZZ_UNDEF_B` pseudo instructions to free up the register allocator.  You don't need to bother with this for this patch if you don't want to but otherwise you'll need to handle `AArch64shadd_p` and `int_aarch64_sve_shadd` as separate patterns because they have different behaviour.

For an example of how we normally handle this see `MUL_ZPmZ` verses `MUL_ZPZZ`.


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

https://reviews.llvm.org/D131875