[PATCH] D126322: [ARM] Recognize t2LoopEnd for software pipelining

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:407
+      Cond.push_back(I->getOperand(0));
+      Cond.push_back(MachineOperand::CreateImm(0)); // So it becomes 3 operands
     } else {
----------------
Can you add a high-level comment somewhere, maybe above analyzeBranch, that explains the condition - that 2 values is a conditional branch and 3 is a LE.


================
Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:6794
+              .addReg(ARM::NoRegister);
+      (void)NewCmp;
+      Cond.push_back(MachineOperand::CreateImm(ARMCC::EQ));
----------------
This can remove the variable for NewCmp.


================
Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:6848
+        return nullptr;
+      else if (L.getOpcode() == ARM::MVE_VCTP8 ||
+               L.getOpcode() == ARM::MVE_VCTP16 ||
----------------
There is a isVCTP function, which I think can be used here.


================
Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:6856
+    MachineInstr *LoopDec = MRI.getUniqueVRegDef(LoopDecResult);
+    if (LoopDec->getOpcode() != ARM::t2LoopDec)
+      return nullptr;
----------------
`if (!LoopDec || LoopDec->getOpcode()...` would be a little safer.


================
Comment at: llvm/lib/Target/ARM/MVETPAndVPTOptimisationsPass.cpp:409
+  MachineBasicBlock *TBB = nullptr, *FBB = nullptr; // For analyzeBranch.
+  if (!TII->analyzeBranch(*(LoopEnd->getParent()), TBB, FBB, Cond) && !FBB) {
+    // If the LoopEnd falls through, need to insert a t2B to the fall-through
----------------
This needn't have the brackets around LoopEnd->getParent() I don't think


================
Comment at: llvm/test/CodeGen/Thumb2/swp-fixedii-le.mir:3
+# NOTE: cortex-m7 is used to provide scheduling information despite not having LE instructions
+# RUN: llc -mtriple=thumbv7m-none-eabi -mcpu=cortex-m7 -run-pass=pipeliner -o - %s | FileCheck %s --check-prefix=CHECK
+
----------------
Can you make this -mtriple=thumbv8.1-m.main-none-eabi. Otherwise the instructions like t2LoopEnd are technically not valid, and should (but currently won't) be giving verifier errors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126322