[PATCH] D86956: [AArch64] Avoid pairing loads when the base reg is modified

[Congzhe Cao via Phabricator via llvm-commits]

congzhe added inline comments.


================
Comment at: llvm/test/CodeGen/AArch64/aarch64-ldst-modified-baseReg.mir:17
+# CHECK: $x10 = LDURXi $x8, 1 :: (load 8)
+# CHECK: $x10 = LDRXui $x10, 0 :: (load 8)
+# CHECK: RET
----------------
fhahn wrote:
> It might be good to have test cases with stores & non-load instructions modifying the base?
Thanks for this comment, now added test cases where the base register or base address is modified with non-load instructions.
However, the bug in AArch64 Load Store Optimization pass only exists when the base register is updated with load instructions in between two loads for pairing, i.e., 

     ldr x9 [x10]
     ldr x10 [x8]
     ldr x10 [x10, 8].

For other instructions that modify the base register or base address with non-load instructions, the existing pass works correctly. The case that the base register is modified with non-load instructions is handled in lines 1626-1627 of the original AArch64 Load Store Optimization pass:

```    if (!ModifiedRegUnits.available(BaseReg))
      return E;
```
Only when the pattern shown above occurs, this pass fails to handle it because it would hit a `continue` in the for-loop and would not reach lines 1626-1627.

Still, I added these test cases where the base register or base address is modified with non-load instructions, but please note that these test cases already work correctly without the patch since the original AArch64 Load Store Optimization pass handles them well. I did not find similar tests in other test files that's why I added them, but maybe not adding them also makes sense. I'll appreciate it if you could let me know your thoughts.



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

https://reviews.llvm.org/D86956