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

[Jun Bum Lim via llvm-commits]

junbuml added a comment.

Can you please upload the diff with full context. 
http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface


================
Comment at: lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp:189
@@ +188,3 @@
+static bool isAdjacentLdStMerge(MachineInstr *MI, unsigned Limit) {
+  if (!MI->mayStore() &&
+      !MI->mayLoad() &&
----------------
You can use clang-format to check your coding style.

================
Comment at: lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp:195-198
@@ +194,6 @@
+
+  if (Limit == 1)
+    return true;
+
+  return false;
+}
----------------
You can replace this with return Limit == 1; 

================
Comment at: lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp:842
@@ +841,3 @@
+        // memory reference information which treated volatile access.
+        if ((!isAdjacentLdStMerge(MI, Limit) && MI->hasOrderedMemoryRef()) ||
+             TII->isLdStPairSuppressed(MI))
----------------
Do you need to find a paired until you encounter the first memory operation? Or did you intend to check just right next instruction?  

Looks like you try to see just right next instruction. Right? If that's the case you could use Count as well. 


================
Comment at: lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp:1160-1161
@@ -1139,3 +1159,4 @@
+  bool isVolatile = false;
   if (MI->hasOrderedMemoryRef())
-    return false;
+    isVolatile = true;
 
----------------
I think you can move this check in findMatchingInsn().

================
Comment at: lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp:1176-1178
@@ +1175,5 @@
+  if (isVolatile) {
+    // Only check the next instruction, if this is volatile load/store.
+    // We just pass 1 for ScanLimit value. That means we just check the
+    // next instruction for merging.
+    Paired = findMatchingInsn(MBBI, Flags, 1);
----------------
Can you add little bit more detail about why you want to check just the right next instruction ?


http://reviews.llvm.org/D14489