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

[Congzhe Cao via Phabricator via llvm-commits]

congzhe added a comment.

Thanks for all the comments, now addressed all of them @fhahn



================
Comment at: llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp:1572
 
+          // If the BaseReg has been modified, then cannot do the optimization.
+          // for example, in the following pattern
----------------
fhahn wrote:
> then *we* cannot do the optimization?
Thanks, updated accordingly.


================
Comment at: llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp:1578
+          // the first and third ldr cannot be converted to ldp x1, x4, [x2]
+          if (!ModifiedRegUnits.available(BaseReg))
+            return E;
----------------
fhahn wrote:
> can we hoist the check outside of the containing if, i.e. to around line 1562? I think we should be able to bail out once the base reg is modified, because it won't get 'un-modified' so that should not rule out any valid pairs.
> 
> Also, it is safer to do it earlier, otherwise we would need the check for each code path that returns a valid found pair (for example, we would probably also need it around line 1611)
Now hoisted the check outside of the containing if.


================
Comment at: llvm/test/CodeGen/AArch64/aarch64-ldst-modified-baseReg.mir:5
+# into an ldp instruction, and when the base register of the second ldr instruction
+# has been modified in between these two ldr instructions, the convertion should not
+# occur.
----------------
fhahn wrote:
> `convertion` -> `conversion`?
Thanks for the comments, revised accordingly.


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

https://reviews.llvm.org/D86956