[PATCH] D113281: [AArch64][SVE] Generate ASRD instructions for power of 2 signed divides

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm accepted this revision.
paulwalker-arm added a comment.

Thanks @bsmith and sorry for the late review.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:13029
   EVT VT = N->getValueType(0);
+  SDLoc DL(N);
+
----------------
This change is not needed anymore.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:13032
+  // For scalable and fixed types, mark them as cheap so we can handle it much
+  // later. This allows us to handle larger than legal fixed types.
+  if (VT.isScalableVector() || Subtarget->useSVEForFixedLengthVectors())
----------------
This should just be `handle larger than legal types` as it applies to all vector types.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:18763-18764
+    SDValue Res =
+        DAG.getNode(AArch64ISD::SRAD_MERGE_OP1, dl, ContainerVT, Pg, Op0,
+                    DAG.getTargetConstant(Log2_64(SplatVal), dl, MVT::i32));
+    if (Negated)
----------------
Up to you but I think having 
```
SDValue Op1 = convertToScalableVector(DAG, ContainerVT, Op.getOperand(0));
SDValue Op2 = DAG.getTargetConstant(Log2_64(SplatVal), dl, MVT::i32)
```
is cleaner whilst also being more consistent with the `OP1` in the opcode's name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113281