[PATCH] D83377: [ARM] Expand distributing increments to also handle existing pre/post inc instructions.

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMLoadStoreOptimizer.cpp:2577
 }
+static bool isPreIndex(MachineInstr &MI) {
+  switch (MI.getOpcode()) {
----------------
The function name doesn't appear to match that opcodes that it's checking?


================
Comment at: llvm/lib/Target/ARM/ARMLoadStoreOptimizer.cpp:2688
       return false;
-    if (Use.getOperand(BaseOp + 1).getImm() == 0)
+    if (isPreIndex(Use) || isPostIndex(Use))
+      PrePostInc = &Use;
----------------
I'm still a bit concerned with this loop.... Can't we get in a pickle if there are two or more instructions within the loop which use BaseReg with a zero offset? OtherAccesses relies on BaseAccess only being assigned once, right? Don't we also need to ensure that BaseReg isn't assigned between BaseAccess and OtherAccesses?


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

https://reviews.llvm.org/D83377