[PATCH] D77813: [ARM] Distribute MVE post-increments

Sam Parker via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 14 08:33:18 PDT 2020


samparker added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMLoadStoreOptimizer.cpp:2538
+// postinc writeback.
+static int getAddSubOffset(MachineInstr &MI) {
+  switch (MI.getOpcode()) {
----------------
Looks like these helpers should be extracted to ARMBaseInstrInfo, I also have a helper in ARMLowOverheadLoops to get this immediate. And I would have thought that there's already something to legal addressing modes somewhere?


================
Comment at: llvm/lib/Target/ARM/ARMLoadStoreOptimizer.cpp:2614
+  for (auto &Use : MRI->use_nodbg_instructions(Base)) {
+    int BaseOp;
+    if (getAddSubOffset(Use) != 0)
----------------
can be assigned from getBaseOp... on initialisation.


================
Comment at: llvm/lib/Target/ARM/ARMLoadStoreOptimizer.cpp:2616
+    if (getAddSubOffset(Use) != 0)
+      Increment = &Use;
+    else if ((BaseOp = getBaseOperandIndex(Use)) != -1) {
----------------
Could increment be assigned multiple times? And if so, does that matter?


================
Comment at: llvm/lib/Target/ARM/ARMLoadStoreOptimizer.cpp:2617
+      Increment = &Use;
+    else if ((BaseOp = getBaseOperandIndex(Use)) != -1) {
+      if (!Use.getOperand(BaseOp).isReg() ||
----------------
exit early on inverse instead?


================
Comment at: llvm/lib/Target/ARM/ARMLoadStoreOptimizer.cpp:2670
+      }
+    } else if (!DT->dominates(Use, BaseAccess)) {
+      LLVM_DEBUG(
----------------
Do we need this condition..?


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

https://reviews.llvm.org/D77813





More information about the llvm-commits mailing list