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

[Stelios Ioannou via Phabricator via llvm-commits]

stelios-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:2215
+  // instruction is a LDRQpre.
+  if (MI.hasOrderedMemoryRef() && MI.getOpcode() != AArch64::LDRQpre)
     return false;
----------------
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. 


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:2238
+  // to ldp q0, q1, [x11, #32]!
   if (MI.getOperand(1).isReg()) {
     Register BaseReg = MI.getOperand(1).getReg();
----------------
Note for `LDRQpre` instructions it should be `MI.getOperand(2).getReg()`, and also `BaseReg` should be `Register BaseReg = MI.getOperand(2).getReg()`. This can be easily fixed, however it won’t do much because `MI.modifiesRegister(BaseReg, TRI)` will again result to true.  Any suggestions? 


================
Comment at: llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp:978
+  } else
+      assert(Scale==1 && "The scale for non pre/post indexed variants must be 1.");
+
----------------
Mis-indentation. I am going to fix this is in an updated revision. 


================
Comment at: llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp:1267
+           !TII->isLdStPairSuppressed(FirstMI)) ||
+          FirstMI.getOpcode() == AArch64::LDRQpre) &&
          "FirstMI shouldn't get here if either of these checks are true.");
----------------
Ditto. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99272