[PATCH] D14489: [AArch64] Applying load pair optimization for volatile load

[Chad Rosier via llvm-commits]

mcrosier added a comment.

Please add a few test cases.

This seems reasonable, but I would really like to hear from Tim or another reviewer.


================
Comment at: lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp:818
@@ -817,3 +817,3 @@
         // safely transform. Similarly, stop if we see a hint to avoid pairs.
-        if (MI->hasOrderedMemoryRef() || TII->isLdStPairSuppressed(MI))
+        if ((Limit != 1 && MI->hasOrderedMemoryRef()) || TII->isLdStPairSuppressed(MI))
           return E;
----------------
Please create a static helper function with comments suggesting why it's safe to merge adjacent loads/stores that are marked volatile.  Also, I don't think this check is sufficient.  The order of the offsets determine the ordering of the pairs.  For example,

ldr x1, [x0, #4]  <--- volatile
ldr x2, [x0]

would create a ldp with the volatile load being reordering, if I'm not mistaken.

================
Comment at: lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp:1150
@@ +1149,3 @@
+  MachineBasicBlock::iterator Paired;
+  if (isVolatile) {
+    // Only check the next instruction, if this is volatile load.
----------------
Please add a few comments about what's going on here.


http://reviews.llvm.org/D14489