public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Roland McGrath <mcgrathr@google.com>
To: gcc-patches@gcc.gnu.org
Subject: [PATCH] ARM: exclude fixed_regs for stack-alignment save/restore
Date: Thu, 14 Jun 2012 17:23:00 -0000	[thread overview]
Message-ID: <x57jboklg6f9.fsf@frobland.mtv.corp.google.com> (raw)

When the ARM compiler needs to ensure the stack pointer stays aligned
and it's already doing a multi-register push/pop in the prologue and
epilogue, it chooses some arbitrary register to add to the register set
in that push and pop just to increase the size of the stack used by 4
bytes.  This is presumed to be harmless, since some register that is
either call-clobbered or not touched by the function at all is just
getting pushed and then the same value popped into it.  

But if e.g. I use -ffixed-r9 then I think it's a reasonable expectation
that no code is generated that touches r9 in any way, shape, or form.
(My actual concern is a variant target port still in progress, where
the ABI specifies that r9 is reserved, and the system enforces that no
instruction may modify r9.)

I haven't managed to come up with an isolated test case to demonstrate
the bug.  Apparently I just don't understand the stack and register
pressure requirements that make the compiler get into the situation
where it wants to add a random register for alignment padding purposes.

I don't have a setup where I can do a proper regression test for ARM.
(My system has a /usr/arm-linux-gnueabi/include/ but configuring with
--target=arm-linux-gnueabi --with-headers=/usr/arm-linux-gnueabi/include
did not succeed in building libgcc.)

But the change seems pretty obviously correct IMHO.


Thanks,
Roland


gcc/
2012-06-14  Roland McGrath  <mcgrathr@google.com>

	* config/arm/arm.c (arm_get_frame_offsets): Never use a fixed register
	as the extra register to save/restore for stack-alignment padding.

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 092e202..bcfef3e 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -16752,7 +16752,12 @@ arm_get_frame_offsets (void)
 	  else
 	    for (i = 4; i <= (TARGET_THUMB1 ? LAST_LO_REGNUM : 11); i++)
 	      {
-		if ((offsets->saved_regs_mask & (1 << i)) == 0)
+		/* While the gratuitous register save/restore is ordinarily
+		   harmless, if a register is marked as fixed it may be
+		   entirely forbidden by the system ABI to touch it, so we
+		   should avoid those registers.  */
+		if (!fixed_regs[i]
+		    && (offsets->saved_regs_mask & (1 << i)) == 0)
 		  {
 		    reg = i;
 		    break;

             reply	other threads:[~2012-06-14 17:17 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-14 17:23 Roland McGrath [this message]
2012-06-14 20:22 ` Mike Stump
2012-06-14 20:51   ` Roland McGrath
2012-06-15  0:44     ` Roland McGrath
2012-06-16 14:33     ` Richard Sandiford
2012-06-18 15:39       ` Richard Earnshaw
2012-06-18 16:56         ` Roland McGrath
2012-06-20 17:37           ` Roland McGrath
2012-07-17 20:42             ` Roland McGrath
2012-07-20 20:21               ` Roland McGrath
2012-07-24 11:54               ` Richard Earnshaw
2012-07-24 16:21                 ` Roland McGrath

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=x57jboklg6f9.fsf@frobland.mtv.corp.google.com \
    --to=mcgrathr@google.com \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).