Re: [PATCH] ARM: exclude fixed_regs for stack-alignment save/restore
On 17/07/12 21:42, Roland McGrath wrote:
Richard, here is the patch against the current trunk, as I promised
last week in Prague. Please apply.
Done.
I've tweaked the comments slightly, but the functional modification is
unchanged.
R.
Thanks,
Roland
gcc/
2012-07-17 Roland McGrath mcgra...@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 e2f625c..189f71e 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -16121,7 +16121,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 or global 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;
Re: [PATCH] ARM: exclude fixed_regs for stack-alignment save/restore
Thanks muchly!
Re: [PATCH] ARM: exclude fixed_regs for stack-alignment save/restore
ping?
Re: [PATCH] ARM: exclude fixed_regs for stack-alignment save/restore
Richard, here is the patch against the current trunk, as I promised
last week in Prague. Please apply.
Thanks,
Roland
gcc/
2012-07-17 Roland McGrath mcgra...@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 e2f625c..189f71e 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -16121,7 +16121,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 or global 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;
Re: [PATCH] ARM: exclude fixed_regs for stack-alignment save/restore
On Mon, Jun 18, 2012 at 9:34 AM, Roland McGrath mcgra...@google.com wrote: OK then. If you like the original patch, would you like to commit it for me? ping?
Re: [PATCH] ARM: exclude fixed_regs for stack-alignment save/restore
On 16/06/12 13:42, Richard Sandiford wrote: Roland McGrath mcgra...@google.com writes: On Thu, Jun 14, 2012 at 1:13 PM, Mike Stump mikest...@comcast.net wrote: On Jun 14, 2012, at 10:16 AM, Roland McGrath wrote: 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. Also, I can't help but wonder if global_regs is respected. It's clearly not. Making the added condition !fixed_regs[i] !global_regs[i] seems sensible to me. All global registers have to be fixed though. The original seemed fine to me FWIW. Richard Indeed, see globalize_reg() in reginfo.c. R.
Re: [PATCH] ARM: exclude fixed_regs for stack-alignment save/restore
OK then. If you like the original patch, would you like to commit it for me? Thanks, Roland
Re: [PATCH] ARM: exclude fixed_regs for stack-alignment save/restore
On Jun 14, 2012, at 10:16 AM, Roland McGrath wrote: 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. Also, I can't help but wonder if global_regs is respected. In theory, people are allowed to declare global registers, and nothing should be stopping them, though, this is abi breaking, and one does need to recompile the world as I recall to use it, so, most people don't use it and can't use it, but the bare metal people can. Your change looks good to me. I'll note in passing that cse.c does: /* Determine whether register number N is considered a fixed register for the purpose of approximating register costs. It is desirable to replace other regs with fixed regs, to reduce need for non-fixed hard regs. A reg wins if it is either the frame pointer or designated as fixed. */ #define FIXED_REGNO_P(N) \ ((N) == FRAME_POINTER_REGNUM || (N) == HARD_FRAME_POINTER_REGNUM \ || fixed_regs[N] || global_regs[N])
Re: [PATCH] ARM: exclude fixed_regs for stack-alignment save/restore
On Thu, Jun 14, 2012 at 1:13 PM, Mike Stump mikest...@comcast.net wrote: On Jun 14, 2012, at 10:16 AM, Roland McGrath wrote: 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. Also, I can't help but wonder if global_regs is respected. It's clearly not. Making the added condition !fixed_regs[i] !global_regs[i] seems sensible to me. Thanks, Roland
Re: [PATCH] ARM: exclude fixed_regs for stack-alignment save/restore
Here's the version of the change that incorporates Mike's suggestion.
Thanks,
Roland
gcc/
2012-06-14 Roland McGrath mcgra...@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..13771d9 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 or global it
+ may be entirely forbidden by the system ABI to touch it,
+ so we should avoid those registers. */
+ if (!fixed_regs[i] !global_regs[i]
+(offsets-saved_regs_mask (1 i)) == 0)
{
reg = i;
break;
