[PATCH] ARMLoadStoreOptimizer: Create LDRD/STRD on thumb2

[Renato Golin]

With a few nitpicks, looks good to me. John, any comments?


REPOSITORY
  rL LLVM

================
Comment at: lib/Target/ARM/ARMLoadStoreOptimizer.cpp:756
@@ +755,3 @@
+         "Must have interger load or store");
+  unsigned LoadStoreOpcode = isi32Load(Opcode) ? ARM::t2LDRDi8 : ARM::t2STRDi8;
+
----------------
nitpick: I'd have kept isi32Load(Opecode) in a bool IsLoad, and reused it on all cases.

================
Comment at: lib/Target/ARM/ARMLoadStoreOptimizer.cpp:945
@@ -900,1 +944,3 @@
         unsigned RegNum = MO.isUndef() ? UINT_MAX : TRI->getEncodingValue(Reg);
+        bool CanMergeThisToLoadStoreMulti = false;
+        bool CanMergeThisToLoadStoreDouble = false;
----------------
nitpick: variable name too close to CanMergeToLoadStoreMulti, confusing.

================
Comment at: lib/Target/ARM/ARMLoadStoreOptimizer.cpp:964
@@ +963,3 @@
+            CanMergeToLoadStoreMulti &= CanMergeThisToLoadStoreMulti;
+            CanMergeToLoadStoreDouble &= CanMergeThisToLoadStoreDouble;
+            // Track MemOp with latest and earliest position (Positions are
----------------
this is a bit confusing... probably because of the close variable names, but also because of the chain of conditionals. Maybe it'll get better after you renamed the *This* variants to something smaller and more distinct.

================
Comment at: lib/Target/ARM/ARMLoadStoreOptimizer.cpp:1743
@@ +1742,3 @@
+        unsigned Opcode = Merged->getOpcode();
+        if (Opcode == ARM::t2STRDi8 || Opcode == ARM::t2LDRDi8)
+          ; // No merging code yet.
----------------
better to leave if (! ...) here, not an empty block.

http://reviews.llvm.org/D10623

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/