[PATCH] Fix and enable the Load/Store optimisation pass for Thumb1

Renato Golin renato.golin at linaro.org
Thu May 15 06:30:07 PDT 2014


Hi Moritz,

The patch looks good, aside from the few comments. Also, would be good to have store tests as well.

cheers,
--renato
PS: when submitting patches to Phab, use "diff -U99" so that we get a bit of context around the changes.

================
Comment at: lib/Target/ARM/ARMLoadStoreOptimizer.cpp:322
@@ +321,3 @@
+                                   ARMCC::CondCodes Pred, unsigned PredReg) {
+  // Start updating any instructions with immediate offsets. Insert a sub before
+  // the first non-updateable instruction (if any).
----------------
assert(isThumb1);

================
Comment at: lib/Target/ARM/ARMLoadStoreOptimizer.cpp:326
@@ +325,3 @@
+    unsigned Opc = MBBI->getOpcode();
+    if (MBBI->readsRegister(Base)) {
+      int Offset;
----------------
This conditional reverted (and merged with the last on in this block) would reduce the indentation of the whole block below.

================
Comment at: lib/Target/ARM/ARMLoadStoreOptimizer.cpp:338
@@ +337,3 @@
+        Offset = MO.getImm() - WordOffset;
+        if (Offset >= 0)
+          MO.setImm(Offset);
----------------
This block is repeated almost identically. I'd be tempted to make it a switch(Opc) to create the MO, than have a single (perhaps slightly more complex) conditional to set the InsertSub. That could also be merged to the check right after this, and make the whole function half of its size (and easier to read).

================
Comment at: lib/Target/ARM/ARMLoadStoreOptimizer.cpp:521
@@ +520,3 @@
+      if (Base == Regs[I].first)
+        Writeback = false;
+
----------------
break;

http://reviews.llvm.org/D3757






More information about the llvm-commits mailing list