Re: [x86] Fix PR target/48142
On Thu, Mar 31, 2011 at 12:09 PM, Jakub Jelinek wrote:
> On Thu, Mar 31, 2011 at 11:58:29AM +0200, Uros Bizjak wrote:
>> + /* Even if the stack pointer isn't the CFA register, we need to correctly
>> + describe the adjustments made to it, in particular differentiate the
>> + frame-related ones from the frame-unrelated ones. */
>> + if (size > 0)
>> + {
>> + rtx expr = gen_rtx_SEQUENCE (VOIDmode, rtvec_alloc (2));
>> + XVECEXP (expr, 0, 0)
>> + = gen_rtx_SET (VOIDmode, stack_pointer_rtx,
>> + plus_constant (stack_pointer_rtx, -size));
>> + XVECEXP (expr, 0, 1)
>> + = gen_rtx_SET (VOIDmode, stack_pointer_rtx,
>> + plus_constant (stack_pointer_rtx,
>> + PROBE_INTERVAL + dope + size));
>> + add_reg_note (last, REG_FRAME_RELATED_EXPR, expr);
>> + RTX_FRAME_RELATED_P (last) = 1;
>> +
>> + cfun->machine->fs.sp_offset += size;
>> + }
>>
>> Is there a reason why we can't just cancel (+ size and -size) in these
>> two expressions to:
>>
>> XVECEXP (expr, 0, 0)
>> = gen_rtx_SET (VOIDmode, stack_pointer_rtx,
>> plus_constant (stack_pointer_rtx,
>> PROBE_INTERVAL + dope));
>
> Yes, in RTX_FRAME_RELATED_P parallels dwarf2out only looks at
> RTX_FRAME_RELATED_P sets or the first set. See dwarf2out_frame_debug_expr
> for more details.
Jakub, thanks for explanation, I was not aware of this fact.
2011-03-30 Eric Botcazou
PR target/48142
* config/i386/i386.c (ix86_adjust_stack_and_probe): Differentiate
frame-related from frame-unrelated adjustments to the stack pointer.
I guess that patch is OK then.
Thanks,
Uros.
Re: [x86] Fix PR target/48142
On Thu, Mar 31, 2011 at 11:58:29AM +0200, Uros Bizjak wrote:
> + /* Even if the stack pointer isn't the CFA register, we need to correctly
> + describe the adjustments made to it, in particular differentiate the
> + frame-related ones from the frame-unrelated ones. */
> + if (size > 0)
> +{
> + rtx expr = gen_rtx_SEQUENCE (VOIDmode, rtvec_alloc (2));
> + XVECEXP (expr, 0, 0)
> + = gen_rtx_SET (VOIDmode, stack_pointer_rtx,
> +plus_constant (stack_pointer_rtx, -size));
> + XVECEXP (expr, 0, 1)
> + = gen_rtx_SET (VOIDmode, stack_pointer_rtx,
> +plus_constant (stack_pointer_rtx,
> + PROBE_INTERVAL + dope + size));
> + add_reg_note (last, REG_FRAME_RELATED_EXPR, expr);
> + RTX_FRAME_RELATED_P (last) = 1;
> +
> + cfun->machine->fs.sp_offset += size;
> +}
>
> Is there a reason why we can't just cancel (+ size and -size) in these
> two expressions to:
>
> XVECEXP (expr, 0, 0)
> = gen_rtx_SET (VOIDmode, stack_pointer_rtx,
> plus_constant (stack_pointer_rtx,
> PROBE_INTERVAL + dope));
Yes, in RTX_FRAME_RELATED_P parallels dwarf2out only looks at
RTX_FRAME_RELATED_P sets or the first set. See dwarf2out_frame_debug_expr
for more details.
Jakub
Re: [x86] Fix PR target/48142
Hello!
> this is a regression present for x86-64 on mainline and 4.6 branch with the
> options -Os -mpreferred-stack-boundary=5 -fstack-check
> -fno-omit-frame-pointer.
> This improbable combination of options is necessary because you need to have
> stack checking + stack realignment + !ACCUMULATE_OUTGOING_ARGS. In this case,
> the DW_CFA_GNU_args_size CFIs must be correct in spite of the frame pointer.
>
> Tested on {i586,x86_64}-suse-linux, OK for mainline and 4.6 branch?
>
>
> 2011-03-30 Eric Botcazou
>
> PR target/48142
> * config/i386/i386.c (ix86_adjust_stack_and_probe): Differentiate
> frame-related from frame-unrelated adjustments to the stack pointer.
- cfun->machine->fs.sp_offset += size;
+
+ /* Even if the stack pointer isn't the CFA register, we need to correctly
+ describe the adjustments made to it, in particular differentiate the
+ frame-related ones from the frame-unrelated ones. */
+ if (size > 0)
+{
+ rtx expr = gen_rtx_SEQUENCE (VOIDmode, rtvec_alloc (2));
+ XVECEXP (expr, 0, 0)
+ = gen_rtx_SET (VOIDmode, stack_pointer_rtx,
+ plus_constant (stack_pointer_rtx, -size));
+ XVECEXP (expr, 0, 1)
+ = gen_rtx_SET (VOIDmode, stack_pointer_rtx,
+ plus_constant (stack_pointer_rtx,
+ PROBE_INTERVAL + dope + size));
+ add_reg_note (last, REG_FRAME_RELATED_EXPR, expr);
+ RTX_FRAME_RELATED_P (last) = 1;
+
+ cfun->machine->fs.sp_offset += size;
+}
Is there a reason why we can't just cancel (+ size and -size) in these
two expressions to:
XVECEXP (expr, 0, 0)
= gen_rtx_SET (VOIDmode, stack_pointer_rtx,
plus_constant (stack_pointer_rtx,
PROBE_INTERVAL + dope));
Uros.
[x86] Fix PR target/48142
Hi,
this is a regression present for x86-64 on mainline and 4.6 branch with the
options -Os -mpreferred-stack-boundary=5 -fstack-check -fno-omit-frame-pointer.
This improbable combination of options is necessary because you need to have
stack checking + stack realignment + !ACCUMULATE_OUTGOING_ARGS. In this case,
the DW_CFA_GNU_args_size CFIs must be correct in spite of the frame pointer.
Tested on {i586,x86_64}-suse-linux, OK for mainline and 4.6 branch?
2011-03-30 Eric Botcazou
PR target/48142
* config/i386/i386.c (ix86_adjust_stack_and_probe): Differentiate
frame-related from frame-unrelated adjustments to the stack pointer.
2011-03-30 Eric Botcazou
* g++.dg/other/pr48142.C: New test.
--
Eric Botcazou
// PR target/48142
// Testcase by Zdenek Sojka
// { dg-do run { target i?86-*-* x86_64-*-* } }
// { dg-options "-Os -mpreferred-stack-boundary=5 -fstack-check -fno-omit-frame-pointer" }
int main()
{
try { throw 0; }
catch (...) {}
return 0;
}
Index: config/i386/i386.c
===
--- config/i386/i386.c (revision 171716)
+++ config/i386/i386.c (working copy)
@@ -10006,7 +10006,7 @@ ix86_adjust_stack_and_probe (const HOST_
probe that many bytes past the specified size to maintain a protection
area at the botton of the stack. */
const int dope = 4 * UNITS_PER_WORD;
- rtx size_rtx = GEN_INT (size);
+ rtx size_rtx = GEN_INT (size), last;
/* See if we have a constant small number of probes to generate. If so,
that's the easy case. The run-time loop is made up of 11 insns in the
@@ -10046,9 +10046,9 @@ ix86_adjust_stack_and_probe (const HOST_
emit_stack_probe (stack_pointer_rtx);
/* Adjust back to account for the additional first interval. */
- emit_insn (gen_rtx_SET (VOIDmode, stack_pointer_rtx,
- plus_constant (stack_pointer_rtx,
- PROBE_INTERVAL + dope)));
+ last = emit_insn (gen_rtx_SET (VOIDmode, stack_pointer_rtx,
+ plus_constant (stack_pointer_rtx,
+ PROBE_INTERVAL + dope)));
}
/* Otherwise, do the same as above, but in a loop. Note that we must be
@@ -10109,15 +10109,33 @@ ix86_adjust_stack_and_probe (const HOST_
}
/* Adjust back to account for the additional first interval. */
- emit_insn (gen_rtx_SET (VOIDmode, stack_pointer_rtx,
- plus_constant (stack_pointer_rtx,
- PROBE_INTERVAL + dope)));
+ last = emit_insn (gen_rtx_SET (VOIDmode, stack_pointer_rtx,
+ plus_constant (stack_pointer_rtx,
+ PROBE_INTERVAL + dope)));
release_scratch_register_on_entry (&sr);
}
gcc_assert (cfun->machine->fs.cfa_reg != stack_pointer_rtx);
- cfun->machine->fs.sp_offset += size;
+
+ /* Even if the stack pointer isn't the CFA register, we need to correctly
+ describe the adjustments made to it, in particular differentiate the
+ frame-related ones from the frame-unrelated ones. */
+ if (size > 0)
+{
+ rtx expr = gen_rtx_SEQUENCE (VOIDmode, rtvec_alloc (2));
+ XVECEXP (expr, 0, 0)
+ = gen_rtx_SET (VOIDmode, stack_pointer_rtx,
+ plus_constant (stack_pointer_rtx, -size));
+ XVECEXP (expr, 0, 1)
+ = gen_rtx_SET (VOIDmode, stack_pointer_rtx,
+ plus_constant (stack_pointer_rtx,
+ PROBE_INTERVAL + dope + size));
+ add_reg_note (last, REG_FRAME_RELATED_EXPR, expr);
+ RTX_FRAME_RELATED_P (last) = 1;
+
+ cfun->machine->fs.sp_offset += size;
+}
/* Make sure nothing is scheduled before we are done. */
emit_insn (gen_blockage ());
