[PATCH] D99272: [AArch64] Adds a pre-indexed Load/Store optimization for LDRQ-STRQ.

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:2096
   case AArch64::STRQui:
+  case AArch64::STRQpre:
   case AArch64::STRXui:
----------------
We should try and add all the various forms of STR?pre/LDR?pre. Hopefully they all work the same way, with the same operands.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:2215
+  // instruction is a LDRQpre.
+  if (MI.hasOrderedMemoryRef() && MI.getOpcode() != AArch64::LDRQpre)
     return false;
----------------
stelios-arm wrote:
> For `LDRQpre` , `MI.hasOrderedMemoryRef()`results to true because the instruction has no memory reference information, and conservatively assumes it wasn't preserved.  Therefore, I added:  
> ```
> && MI.getOpcode() != AArch64::LDRQpre
> ```
> to ignore it for this instruction. I suppose there is a better way of doing it, but I am not yet sure how. 
Why does the LDRQpre have no memory operand?


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:2223
+  // For Pre-indexed addressing quadword instructions,
+  // the third opperand is the immediate value.
+  bool isPreLdSt = ((MI.getOpcode() == AArch64::STRQpre) ||
----------------
-> operand
Formatting looks a bit off, which might or might not be fixed by just running clang-format on the patch.


================
Comment at: llvm/test/CodeGen/AArch64/strqpre-strqui-merge.mir:64
+    liveins: $q0, $q1, $x0
+    early-clobber $x0 = STPQpre killed renamable $q0, killed renamable $q1, renamable $x0, 2 :: (store 16 into %ir.p0), (store 16 into %ir.p1)
+    RET undef $lr, implicit $x0
----------------
This should presumably be the before-STPQpre code, that is then converted to a STPQpre by the pass.

You can often remove a lot of the stuff above, like the frameInfo and all the regBankSelected stuff.

And there is an update_mir_test_checks for generating check lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99272