[PATCH] D60708: [ARM] Code-generation infrastructure for MVE.

[Tim Northover via Phabricator via llvm-commits]

t.p.northover added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:808-809
 
+void llvm::addUnpredicatedMveVpredNOp(MachineInstrBuilder &MIB)
+{
+  MIB.addImm(ARMVCC::None);
----------------
Braces go on the same line as the ) in LLVM style.


================
Comment at: llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp:1284-1286
+      if (RHSC % (1 << Shift) == 0 &&
+          RHSC >= -(0x7F << Shift) &&
+          RHSC <= +(0x7F << Shift)) {
----------------
I think this would be clearer if you used the `isInt<7>` functions from `MathExtras.h`.


================
Comment at: llvm/lib/Target/ARM/Thumb2ITBlockPass.cpp:208-209
+
+    if ((CC = getITInstrPredicate(*MI, PredReg)) != ARMCC::AL) {
+      Defs.clear();
+      Uses.clear();
----------------
I think this function would be a good place to assert that all instructions have at most one kind of predication.


================
Comment at: llvm/lib/Target/ARM/Thumb2ITBlockPass.cpp:300
+      ++MBBI;
+      continue;
+    }
----------------
This `continue` isn't needed with the new control flow (and is inconsistent with the other branches).


================
Comment at: llvm/lib/Target/ARM/Thumb2InstrInfo.cpp:694
+
+int llvm::findFirstVPTPredOperandIdx(const MachineInstr &MI) {
+  const MCInstrDesc &MCID = MI.getDesc();
----------------
Possibly better as a static helper; then you don't have to explain it at all in the documentation comments in the header.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60708