[PATCH] Fix return sequence on armv4 thumb

Renato Golin renato.golin at linaro.org
Fri Aug 1 04:19:18 PDT 2014


Hi Jon,

See my comments inline.

Also, please add some tests with ARMv4T and ARMv5T variants, checking for their specific returns, using FileCheck.

Thanks!
--renato

================
Comment at: llvm/lib/Target/ARM/Thumb1FrameLowering.cpp:474
@@ -472,1 +473,3 @@
         continue;
+      IsReturn = true;
+      if (STI.hasV4TOps() && !STI.hasV5TOps())
----------------
Since you're already creating a new boolean, why not make it IsV4TReturn and set it inside the conditional below, so that your special lowering only checks for a flag and not a bunch of them.

================
Comment at: llvm/lib/Target/ARM/Thumb1FrameLowering.cpp:475
@@ +474,3 @@
+      IsReturn = true;
+      if (STI.hasV4TOps() && !STI.hasV5TOps())
+        continue;
----------------
A comment here hinting at the block you just added below would be nice. Something like:

    // ARMv4T requires BX, see below

================
Comment at: llvm/lib/Target/ARM/Thumb1FrameLowering.cpp:495
@@ +494,3 @@
+  //   POP {r3}
+  //   BX r3
+  if (IsReturn && STI.hasV4TOps() && !STI.hasV5TOps()) {
----------------
This is probably going to create two POPs. I think a better way would be to detect the LR in the loop above and change it from PC to R3 and just add the BX magic here. It should be safe to assume R3 is fine, but you can add a check if you want.

http://reviews.llvm.org/D4748






More information about the llvm-commits mailing list