[PATCH] D63430: [ARM] Add support for the MVE long shift instructions

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:5537
+  if (ST->hasMVEIntegerOps() &&
+      (ShOpc == ISD::SRA || ShOpc == ISD::SRL || ShOpc == ISD::SHL)) {
+    SDValue ShAmt = N->getOperand(1);
----------------
Unnecessary opcode checks, the above assert is good enough.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:5553
+      if (!Con)
+        // If the shift amount is in a register then negate it and perform an
+        // lsll
----------------
This isn't immediately obvious to me why this is correct, so could you please expand in the comment?


================
Comment at: llvm/lib/Target/ARM/ARMInstrInfo.td:181
 
+def ARMasrl          : SDNode<"ARMISD::ASRL", SDT_ARMIntShiftParts, [SDNPOutGlue]>;
+def ARMlsrl          : SDNode<"ARMISD::LSRL", SDT_ARMIntShiftParts, [SDNPOutGlue]>;
----------------
Why do we have Glue flags here?


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

https://reviews.llvm.org/D63430