[PATCH] D99272: [AArch64] Adds a pre-indexed paired Load/Store optimization for LDR-STR.

Fri Apr 9 04:44:05 PDT 2021

stelios-arm marked 2 inline comments as done.
stelios-arm added a comment.

In D99272#2647947 <https://reviews.llvm.org/D99272#2647947>, @SjoerdMeijer wrote:

> Hi Stelios, many thanks for putting this together, good stuff. 
> I will do a code-review a bit later, but as there's potential for some corner cases here, first a testing question. Did you do a bootstrap build and e.g. ran the llvm test suite?

This was done for the second revision.

================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:2215
+  // instruction is a LDRQpre.
+  if (MI.hasOrderedMemoryRef() && MI.getOpcode() != AArch64::LDRQpre)
     return false;
----------------
dmgreen wrote:
> 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?
I added an explanation in the new revision (Point 5). This is going to be addressed in another patch and this patch will be updated accordingly. 

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

https://reviews.llvm.org/D99272