[PATCH] D128065: [AArch64][SVE] Fold target specific ext/trunc nodes into loads/stores

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm accepted this revision.
paulwalker-arm added a comment.
This revision is now accepted and ready to land.

I've added a suggested improvement if you agree, but otherwise looks good.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17144
+  if (N->getOperand(0).getOpcode() == ISD::MLOAD &&
+      N->getOpcode() == AArch64ISD::UUNPKLO && N->getValueType(0).isInteger()) {
+    MaskedLoadSDNode *MLD = cast<MaskedLoadSDNode>(N->getOperand(0));
----------------
This is not necessary because `AArch64ISD::UUNPKLO` only supports integer types.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17523-17530
+      Value.getValueType().isInteger()) {
+    Value = Value.getOperand(0);
+    if (Value.getOpcode() == ISD::BITCAST) {
+      EVT HalfVT =
+          Value.getValueType().getHalfNumVectorElementsVT(*DAG.getContext());
+      EVT InVT = Value.getOperand(0).getValueType();
+
----------------
Whilst this works, you don't need to restrict the combine like this.  Here you're checking the stored elements all come from `UZP1`'s first operand.  You could do this using:
```
ValueVT.isInteger() && ValueVT != MVT::nxv2i64) {
EVT HalfVT = ValueVT.getHalfNumVectorElementsVT(*DAG.getContext());
EVT InVT = HalfVT.widenIntegerVectorElementType(*DAG.getContext());
```
followed by your current `NumElts` check. Then just before the `getMaskedStore()` create a fresh bitcast (e.g. `Value = bitcast(Value.getOperand(0) to InVT)`.  This means you don't really care what `UZP1`'s first operand is and you might catch more cases.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128065