This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix -mno-accumulate-outgoing-args -fasynchronous-unwind-tables (PR rtl-optimization/36419, take 2)
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Jason Merrill <jason at redhat dot com>, Eric Botcazou <ebotcazou at adacore dot com>, gcc-patches at gcc dot gnu dot org, Xuepeng Guo <xuepeng dot guo at intel dot com>, Joey Ye <joey dot ye at intel dot com>
- Date: Fri, 18 Jul 2008 07:17:29 -0700
- Subject: Re: [PATCH] Fix -mno-accumulate-outgoing-args -fasynchronous-unwind-tables (PR rtl-optimization/36419, take 2)
- References: <20080611133108.GM8133@devserv.devel.redhat.com> <200806151630.27801.ebotcazou@adacore.com> <20080615155952.GH8133@devserv.devel.redhat.com> <200806152358.59302.ebotcazou@adacore.com> <20080619194814.GT8133@devserv.devel.redhat.com>
On Thu, Jun 19, 2008 at 03:48:14PM -0400, Jakub Jelinek wrote:
> On Sun, Jun 15, 2008 at 11:58:59PM +0200, Eric Botcazou wrote:
> > > I agree that representing it in RTL rather than on the side would be
> > > cleaner. But from what I can see, most of the targets set
> > > REG_FRAME_RELATED_EXPR to something that doesn't have RTX_FRAME_RELATED_P,
> > > so dwarf2out.c would be confused about them. Furthermore, it is
> > > possible that some target already has different REG_FRAME_RELATED_EXPR
> > > on it (maybe not, haven't read all the backends to see what they set
> > > REG_FRAME_RELATED_EXPR on).
> >
> > My understanding is that you can always add a new CFI, even if there is
> > already one, just make a PARALLEL if needed. So we could devise a new
> > RTL representation for DW_CFA_GNU_args_size and use it there. But I
> > cannot really approve anything in this area so discussing this with a
> > maintainer would probably be more productive. :-)
>
> Actually, it seems it is possible to use REG_FRAME_RELATED_EXPR
> here, as dwarf2out documents, for compatibility reasons the first set
> in PARALLEL/SEQUENCE isn't checked from RTX_FRAME_RELATED_P, but all
> others are.
>
> Here is what I've bootstrapped/regtested on i686-linux. It shouldn't
> fix just the case of CSA pass merging post-prologue stack adjustment
> into prologue stack adjustment, but also merging two prologue stack
> adjustments if the first one has REG_FRAME_RELATED_EXPR on it.
>
> Ok for trunk/4.3?
>
> 2008-06-19 Jakub Jelinek <jakub@redhat.com>
>
> PR rtl-optimization/36419
> * combine-stack-adj.c (adjust_frame_related_expr): New function.
> (combine_stack_adjustments_for_block): Call it if needed. Delete
> correct insn.
> * dwarf2out.c (dwarf2out_frame_debug_expr): Adjust
> DW_CFA_GNU_args_size if CSA pass merged some adjustments into
> prologue sp adjustment.
>
> * g++.dg/eh/async-unwind1.C: New test.
>
Hi Jakub, Jason,
Your patch caused a regresion on stack branch. The problem is
in adjust_frame_related_expr, there are
else
{
rtx expr = copy_rtx (single_set_for_csa (last_sp_set));
XEXP (SET_SRC (expr), 1)
= GEN_INT (INTVAL (XEXP (SET_SRC (expr), 1)) - this_adjust);
RTX_FRAME_RELATED_P (expr) = 1;
XVECEXP (new_expr, 0, 0) = expr;
}
When you combine a stack adjustment insn
(insn:HI 6 5 84 2
/export/gnu/src/gcc-stack/gcc/gcc/testsuite/g++.dg/torture/stackalign/unwind-2.C:11
(parallel [
(set (reg/f:SI 7 sp)
(plus:SI (reg/f:SI 7 sp)
(const_int -32 [0xffffffffffffffe0])))
(clobber (reg:CC 17 flags))
]) 284 {*addsi_1} (expr_list:REG_UNUSED (reg:CC 17 flags)
(nil)))
with a frame related insn:
(insn/f 90 89 91 2
/export/gnu/src/gcc-stack/gcc/gcc/testsuite/g++.dg/torture/stackalign/unwind-2.C:8
(parallel [
(set (reg/f:SI 7 sp)
(plus:SI (reg/f:SI 7 sp)
(const_int -184 [0xffffffffffffff48])))
(clobber (reg:CC 17 flags))
(clobber (mem:BLK (scratch) [0 A8]))
]) 890 {pro_epilogue_adjust_stack_1} (expr_list:REG_UNUSED
(reg:CC 17 flags)
(nil)))
you got,
(sequence [
(set/f (reg/f:SI 7 sp)
(plus:SI (reg/f:SI 7 sp)
(const_int -152 [0xffffffffffffff68])))
(set/f (reg/f:SI 7 sp)
(plus:SI (reg/f:SI 7 sp)
(const_int -32 [0xffffffffffffffe0])))
])
Since the other insn isn't frame related, you have one sequence
in prologue with one insn isn't marke as frame related. It leads
to the wrong unwind info. This patch makes sure that both insns
are marked as frame related. OK for trunk?
Thanks.
H.J.
----
2008-07-18 H.J. Lu <hongjiu.lu@intel.com>
Xuepeng Guo <xuepeng.guo@intel.com>
* combine-stack-adj.c (adjust_frame_related_expr): Set the
new insn frame related when it is combined with a frame related
insn.
--- gcc/combine-stack-adj.c.frame 2008-07-16 22:06:26.000000000 -0700
+++ gcc/combine-stack-adj.c 2008-07-18 06:59:38.000000000 -0700
@@ -281,10 +281,13 @@ adjust_frame_related_expr (rtx last_sp_s
{
rtx note = find_reg_note (last_sp_set, REG_FRAME_RELATED_EXPR, NULL_RTX);
rtx new_expr = NULL_RTX;
+ unsigned int frame_related;
if (note == NULL_RTX && RTX_FRAME_RELATED_P (insn))
return;
+ frame_related = RTX_FRAME_RELATED_P (insn);
+
if (note
&& GET_CODE (XEXP (note, 0)) == SEQUENCE
&& XVECLEN (XEXP (note, 0), 0) >= 2)
@@ -323,13 +326,14 @@ adjust_frame_related_expr (rtx last_sp_s
= GEN_INT (INTVAL (XEXP (SET_SRC (expr), 1)) - this_adjust);
RTX_FRAME_RELATED_P (expr) = 1;
XVECEXP (new_expr, 0, 0) = expr;
+ frame_related = 1;
}
}
XVECEXP (new_expr, 0, XVECLEN (new_expr, 0) - 1)
= copy_rtx (single_set_for_csa (insn));
RTX_FRAME_RELATED_P (XVECEXP (new_expr, 0, XVECLEN (new_expr, 0) - 1))
- = RTX_FRAME_RELATED_P (insn);
+ = frame_related;
if (note)
XEXP (note, 0) = new_expr;
else