[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