diff mbox

[ARM] Fix PR63718, Thumb1 bootstrap -- disable fuse-caller-save for Thumb1

Message ID 546DD65F.3090906@mentor.com
State New
Headers show

Commit Message

Tom de Vries Nov. 20, 2014, 11:54 a.m. UTC
Richard,

This patch fixes PR63718, which currently breaks Thumb1 bootstrap.

The problem is that in Thumb1 mode, we emit the epilogue in RTL, but the last 
insn - epilogue_insns - does not accurately model the corresponding insns
emitted in the asm file. F.i., the asm file may contain an insn:
...
   pop     {r0}
....
while the corresponding RTL pattern looks like this:
...
(jump_insn (unspec_volatile [
             (return)
          ] VUNSPEC_EPILOGUE))
...

As a consequence, the epilogue may clobber registers without fuse-caller-save 
being able to analyze that.

Adding the missing clobbers to epilogue_insns is not trivial, and probably not a 
good idea for stage3. The patch works around the problem by disabling 
fuse-caller-save in Thumb1 mode.

Build and reg-tested on arm-none-eabi.

OK for stage3?

Thanks,
- Tom

Comments

Joey Ye Nov. 28, 2014, 1:52 a.m. UTC | #1
This work around brings thumb1 bootstrap back. Though I cannot
approve, I would like it in stage 3. A complete fix in thumb1 pattern
will be welcomed.

Thanks,
Joey

On Thu, Nov 20, 2014 at 7:54 PM, Tom de Vries <Tom_deVries@mentor.com> wrote:
> Richard,
>
> This patch fixes PR63718, which currently breaks Thumb1 bootstrap.
>
> The problem is that in Thumb1 mode, we emit the epilogue in RTL, but the
> last insn - epilogue_insns - does not accurately model the corresponding
> insns
> emitted in the asm file. F.i., the asm file may contain an insn:
> ...
>   pop     {r0}
> ....
> while the corresponding RTL pattern looks like this:
> ...
> (jump_insn (unspec_volatile [
>             (return)
>          ] VUNSPEC_EPILOGUE))
> ...
>
> As a consequence, the epilogue may clobber registers without
> fuse-caller-save being able to analyze that.
>
> Adding the missing clobbers to epilogue_insns is not trivial, and probably
> not a good idea for stage3. The patch works around the problem by disabling
> fuse-caller-save in Thumb1 mode.
>
> Build and reg-tested on arm-none-eabi.
>
> OK for stage3?
>
> Thanks,
> - Tom
Tom de Vries Dec. 2, 2014, 8:25 a.m. UTC | #2
On 20-11-14 12:54, Tom de Vries wrote:
> Richard,
>
> This patch fixes PR63718, which currently breaks Thumb1 bootstrap.
>
> The problem is that in Thumb1 mode, we emit the epilogue in RTL, but the last
> insn - epilogue_insns - does not accurately model the corresponding insns
> emitted in the asm file. F.i., the asm file may contain an insn:
> ...
>    pop     {r0}
> ....
> while the corresponding RTL pattern looks like this:
> ...
> (jump_insn (unspec_volatile [
>              (return)
>           ] VUNSPEC_EPILOGUE))
> ...
>
> As a consequence, the epilogue may clobber registers without fuse-caller-save
> being able to analyze that.
>
> Adding the missing clobbers to epilogue_insns is not trivial, and probably not a
> good idea for stage3. The patch works around the problem by disabling
> fuse-caller-save in Thumb1 mode.
>
> Build and reg-tested on arm-none-eabi.
>
> OK for stage3?
>

Ping.

Thanks,
- Tom
Ramana Radhakrishnan Dec. 2, 2014, 12:18 p.m. UTC | #3
On 20/11/14 11:54, Tom de Vries wrote:
> Richard,
>
> This patch fixes PR63718, which currently breaks Thumb1 bootstrap.
>
> The problem is that in Thumb1 mode, we emit the epilogue in RTL, but the last
> insn - epilogue_insns - does not accurately model the corresponding insns
> emitted in the asm file. F.i., the asm file may contain an insn:
> ...
>     pop     {r0}
> ....
> while the corresponding RTL pattern looks like this:
> ...
> (jump_insn (unspec_volatile [
>               (return)
>            ] VUNSPEC_EPILOGUE))
> ...
>
> As a consequence, the epilogue may clobber registers without fuse-caller-save
> being able to analyze that.
>
> Adding the missing clobbers to epilogue_insns is not trivial, and probably not a
> good idea for stage3. The patch works around the problem by disabling
> fuse-caller-save in Thumb1 mode.
>
> Build and reg-tested on arm-none-eabi.
>
> OK for stage3?

This is OK, can you make sure that a bug is created to reflect the work 
required to make this work in the backend.

Sorry about the delay in review.

regards
Ramana

>
> Thanks,
> - Tom
>
diff mbox

Patch

2014-11-20  Tom de Vries  <tom@codesourcery.com>

	PR rtl-optimization/63718
	* config/arm/arm.c (arm_option_override): Disable fuse-caller-save for
	Thumb1.

Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c (revision 217730)
+++ gcc/config/arm/arm.c (working copy)
@@ -3105,6 +3105,18 @@  arm_option_override (void)
       && (!arm_arch7 || !current_tune->prefer_ldrd_strd))
     flag_schedule_fusion = 0;
 
+  /* In Thumb1 mode, we emit the epilogue in RTL, but the last insn
+     - epilogue_insns - does not accurately model the corresponding insns
+     emitted in the asm file.  In particular, see the comment in thumb_exit
+     'Find out how many of the (return) argument registers we can corrupt'.
+     As a consequence, the epilogue may clobber registers without
+     fuse-caller-save finding out about it.  Therefore, disable fuse-caller-save
+     in Thumb1 mode.
+     TODO: Accurately model clobbers for epilogue_insns and reenable
+     fuse-caller-save.  */
+  if (TARGET_THUMB1)
+    flag_use_caller_save = 0;
+
   /* Register global variables with the garbage collector.  */
   arm_add_gc_roots ();
 }