* [PATCH] EH_USES df fix (PR target/37378)
@ 2008-10-27 15:24 Jakub Jelinek
2008-10-27 21:14 ` [PATCH] EH_USES df fix (PR target/37378, take 2) Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2008-10-27 15:24 UTC (permalink / raw)
To: Kenneth Zadeck, Ian Lance Taylor; +Cc: gcc-patches
Hi!
Up to 4.1 flow.c used EH_USES in 2 places, to mark registers used on EH
edges and registers used after by bb's without successors (that means
usually noreturn calls). While the epilogue explicitly uses all EH_USES
registers (it restores various special registers from the values saved in
EH_USES registers and the special registers are said to be live at the end
of epilogue), with noreturn calls the epilogue isn't reached, yet we still
need EH_USES registers to be live from the insn which saved special register
into a particular EH_USES register until the last insn in the function
(in this case the noreturn call). We need them for 2 reasons:
1) for every insn that might throw externally (i.e. not be caught within
the same function, but parent etc.), the EH_USES registers are used
by the unwinder
2) for debugging purposes, we need those registers to be live at every
single insn within the function once the special register has been
clobbered
While for 1) we could add explicit artificial uses to every such potentially
throwing insn, for 2) it would be a very bad idea. And even for 1) it would
be IMHO an overkill, as the insns in prologue that save special regs into
EH_USES registers are all RTX_FRAME_RELATED_P and thus can't be simply
removed as unneeded. So it is IMHO enough to say EH_USES registers
are used by top of BBs with incoming EH edges (this has been already done in
df) and by noreturn calls. Otherwise we risk that the saving insn in the
prologue is moved beyond a branch to bb's ending with noreturn call, as
can be seen in miscompiled _Jv_divI (see PR).
The other hunk just removes an unnecessary nop - EH_USES is only defined
(to non-0) on ia64 and only if reload_completed, but it has been used
inside of if (!reload_completed), so it actually never could make any
difference.
Bootstrapped/regtested on ia64-linux, libjava make check now looks
much better than before:
FAIL: getlocalvartable output
FAIL: Throw_3 -O3 output - source compiled test
FAIL: Throw_3 -O3 -findirect-dispatch output - source compiled test
Ok for trunk?
2008-10-27 Jakub Jelinek <jakub@redhat.com>
PR target/37378
* df-scan.c (df_get_call_refs): For unconditional noreturn
calls add EH_USES regs as artificial uses.
(df_get_entry_block_def_set): Don't handle EH_USES here.
--- gcc/df-scan.c.jj 2008-10-14 07:58:50.000000000 -0400
+++ gcc/df-scan.c 2008-10-27 07:26:20.000000000 -0400
@@ -3406,7 +3406,22 @@ df_get_call_refs (struct df_collection_r
}
BITMAP_FREE (defs_generated);
- return;
+
+#ifdef EH_USES
+ if ((flags & DF_REF_CONDITIONAL) == 0
+ && find_reg_note (insn_info->insn, REG_NORETURN, 0))
+ {
+ unsigned int i;
+ /* This code is putting in an artificial ref for the use at the
+ BOTTOM of the block with noreturn call, as EH_USES registers need
+ to be live until epilogue or noreturn call, for debugging purposes
+ as well as any insns that might throw. */
+ for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
+ if (EH_USES (i))
+ df_ref_record (DF_REF_ARTIFICIAL, collection_rec, regno_reg_rtx[i],
+ NULL, bb, NULL, DF_REF_REG_USE, 0, -1, -1, 0);
+ }
+#endif
}
/* Collect all refs in the INSN. This function is free of any
@@ -3826,16 +3841,6 @@ df_get_entry_block_def_set (bitmap entry
/* These registers are live everywhere. */
if (!reload_completed)
{
-#ifdef EH_USES
- /* The ia-64, the only machine that uses this, does not define these
- until after reload. */
- for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
- if (EH_USES (i))
- {
- bitmap_set_bit (entry_block_defs, i);
- }
-#endif
-
#if FRAME_POINTER_REGNUM != ARG_POINTER_REGNUM
/* Pseudos with argument area equivalences may require
reloading via the argument pointer. */
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] EH_USES df fix (PR target/37378, take 2)
2008-10-27 15:24 [PATCH] EH_USES df fix (PR target/37378) Jakub Jelinek
@ 2008-10-27 21:14 ` Jakub Jelinek
2008-10-27 23:55 ` Kenneth Zadeck
2008-10-28 11:27 ` Eric Botcazou
0 siblings, 2 replies; 6+ messages in thread
From: Jakub Jelinek @ 2008-10-27 21:14 UTC (permalink / raw)
To: Kenneth Zadeck; +Cc: Ian Lance Taylor, gcc-patches
Hi!
As pointed out by Hans-Peter Nilsson, we want to make EH_USES regs live even
if some code path ends with an endless loop, the insns in between still
might throw, or during debugging the debugger (or something called from
async signal handler) might want to do a backtrace). The patch I've posted
earlier doesn't cure e.g.
extern int foo (int, int);
extern int bar (void);
int
baz (int x, int y)
{
if (__builtin_expect (y == 0, 0))
{
foo (x + y, x * y);
bar ();
while (1)
;
}
if (x == (int) 0x80000000L && y == -1)
return x;
return x / y;
}
at -O2 - rp is saved into a local register only if y != 0.
The following alternative patch makes EH_USES registers just live
everywhere, which IMHO matches best what we really want - let those
registers stay live through the whole function.
Bootstrapped/regtested on ia64-linux, ok for trunk?
2008-10-27 Jakub Jelinek <jakub@redhat.com>
PR target/37378
* df-scan.c (df_bb_refs_collect): Don't handle EH_USES here.
(df_get_entry_block_def_set): Neither here.
(df_get_regular_block_artificial_uses): Add EH_USES registers.
--- gcc/df-scan.c.jj 2008-10-14 07:58:50.000000000 -0400
+++ gcc/df-scan.c 2008-10-27 14:13:41.000000000 -0400
@@ -3555,29 +3555,6 @@ df_bb_refs_collect (struct df_collection
}
#endif
-
-#ifdef EH_USES
- if (bb_has_eh_pred (bb))
- {
- unsigned int i;
- /* This code is putting in an artificial ref for the use at the
- TOP of the block that receives the exception. It is too
- cumbersome to actually put the ref on the edge. We could
- either model this at the top of the receiver block or the
- bottom of the sender block.
-
- The bottom of the sender block is problematic because not all
- out-edges of a block are eh-edges. However, it is true
- that all edges into a block are either eh-edges or none of
- them are eh-edges. Thus, we can model this at the top of the
- eh-receiver for all of the edges at once. */
- for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
- if (EH_USES (i))
- df_ref_record (DF_REF_ARTIFICIAL, collection_rec, regno_reg_rtx[i], NULL,
- bb, NULL, DF_REF_REG_USE, DF_REF_AT_TOP, -1, -1, 0);
- }
-#endif
-
/* Add the hard_frame_pointer if this block is the target of a
non-local goto. */
if (bb->flags & BB_NON_LOCAL_GOTO_TARGET)
@@ -3667,6 +3644,8 @@ df_bb_refs_record (int bb_index, bool sc
static void
df_get_regular_block_artificial_uses (bitmap regular_block_artificial_uses)
{
+ unsigned int i;
+
bitmap_clear (regular_block_artificial_uses);
if (reload_completed)
@@ -3702,6 +3681,20 @@ df_get_regular_block_artificial_uses (bi
}
/* The all-important stack pointer must always be live. */
bitmap_set_bit (regular_block_artificial_uses, STACK_POINTER_REGNUM);
+
+#ifdef EH_USES
+ /* EH_USES registers are used:
+ 1) at all insns that might throw (calls or with -fnon-call-exceptions
+ trapping insns)
+ 2) in all EH edges
+ 3) to support backtraces and/or debugging, anywhere between their
+ initialization and where they the saved registers are restored
+ from them, including the cases where we don't reach the epilogue
+ (noreturn call or infinite loop). */
+ for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
+ if (EH_USES (i))
+ bitmap_set_bit (regular_block_artificial_uses, i);
+#endif
}
@@ -3826,16 +3819,6 @@ df_get_entry_block_def_set (bitmap entry
/* These registers are live everywhere. */
if (!reload_completed)
{
-#ifdef EH_USES
- /* The ia-64, the only machine that uses this, does not define these
- until after reload. */
- for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
- if (EH_USES (i))
- {
- bitmap_set_bit (entry_block_defs, i);
- }
-#endif
-
#if FRAME_POINTER_REGNUM != ARG_POINTER_REGNUM
/* Pseudos with argument area equivalences may require
reloading via the argument pointer. */
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] EH_USES df fix (PR target/37378, take 2)
2008-10-27 21:14 ` [PATCH] EH_USES df fix (PR target/37378, take 2) Jakub Jelinek
@ 2008-10-27 23:55 ` Kenneth Zadeck
2008-10-28 11:27 ` Eric Botcazou
1 sibling, 0 replies; 6+ messages in thread
From: Kenneth Zadeck @ 2008-10-27 23:55 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Ian Lance Taylor, gcc-patches
Jakub Jelinek wrote:
> Hi!
>
> As pointed out by Hans-Peter Nilsson, we want to make EH_USES regs live even
> if some code path ends with an endless loop, the insns in between still
> might throw, or during debugging the debugger (or something called from
> async signal handler) might want to do a backtrace). The patch I've posted
> earlier doesn't cure e.g.
>
>
i seriously doubt that this patch will bootstrap on a non ia-64 target,
but once you fix that, the patch is ok. (the problem is that the
declaration of i needs to be in an ifdef EH_USES.)
> extern int foo (int, int);
> extern int bar (void);
>
> int
> baz (int x, int y)
> {
> if (__builtin_expect (y == 0, 0))
> {
> foo (x + y, x * y);
> bar ();
> while (1)
> ;
> }
>
> if (x == (int) 0x80000000L && y == -1)
> return x;
>
> return x / y;
> }
> at -O2 - rp is saved into a local register only if y != 0.
> The following alternative patch makes EH_USES registers just live
> everywhere, which IMHO matches best what we really want - let those
> registers stay live through the whole function.
>
> Bootstrapped/regtested on ia64-linux, ok for trunk?
>
> 2008-10-27 Jakub Jelinek <jakub@redhat.com>
>
> PR target/37378
> * df-scan.c (df_bb_refs_collect): Don't handle EH_USES here.
> (df_get_entry_block_def_set): Neither here.
> (df_get_regular_block_artificial_uses): Add EH_USES registers.
>
> --- gcc/df-scan.c.jj 2008-10-14 07:58:50.000000000 -0400
> +++ gcc/df-scan.c 2008-10-27 14:13:41.000000000 -0400
> @@ -3555,29 +3555,6 @@ df_bb_refs_collect (struct df_collection
> }
> #endif
>
> -
> -#ifdef EH_USES
> - if (bb_has_eh_pred (bb))
> - {
> - unsigned int i;
> - /* This code is putting in an artificial ref for the use at the
> - TOP of the block that receives the exception. It is too
> - cumbersome to actually put the ref on the edge. We could
> - either model this at the top of the receiver block or the
> - bottom of the sender block.
> -
> - The bottom of the sender block is problematic because not all
> - out-edges of a block are eh-edges. However, it is true
> - that all edges into a block are either eh-edges or none of
> - them are eh-edges. Thus, we can model this at the top of the
> - eh-receiver for all of the edges at once. */
> - for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
> - if (EH_USES (i))
> - df_ref_record (DF_REF_ARTIFICIAL, collection_rec, regno_reg_rtx[i], NULL,
> - bb, NULL, DF_REF_REG_USE, DF_REF_AT_TOP, -1, -1, 0);
> - }
> -#endif
> -
> /* Add the hard_frame_pointer if this block is the target of a
> non-local goto. */
> if (bb->flags & BB_NON_LOCAL_GOTO_TARGET)
> @@ -3667,6 +3644,8 @@ df_bb_refs_record (int bb_index, bool sc
> static void
> df_get_regular_block_artificial_uses (bitmap regular_block_artificial_uses)
> {
> + unsigned int i;
> +
> bitmap_clear (regular_block_artificial_uses);
>
> if (reload_completed)
> @@ -3702,6 +3681,20 @@ df_get_regular_block_artificial_uses (bi
> }
> /* The all-important stack pointer must always be live. */
> bitmap_set_bit (regular_block_artificial_uses, STACK_POINTER_REGNUM);
> +
> +#ifdef EH_USES
> + /* EH_USES registers are used:
> + 1) at all insns that might throw (calls or with -fnon-call-exceptions
> + trapping insns)
> + 2) in all EH edges
> + 3) to support backtraces and/or debugging, anywhere between their
> + initialization and where they the saved registers are restored
> + from them, including the cases where we don't reach the epilogue
> + (noreturn call or infinite loop). */
> + for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
> + if (EH_USES (i))
> + bitmap_set_bit (regular_block_artificial_uses, i);
> +#endif
> }
>
>
> @@ -3826,16 +3819,6 @@ df_get_entry_block_def_set (bitmap entry
> /* These registers are live everywhere. */
> if (!reload_completed)
> {
> -#ifdef EH_USES
> - /* The ia-64, the only machine that uses this, does not define these
> - until after reload. */
> - for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
> - if (EH_USES (i))
> - {
> - bitmap_set_bit (entry_block_defs, i);
> - }
> -#endif
> -
> #if FRAME_POINTER_REGNUM != ARG_POINTER_REGNUM
> /* Pseudos with argument area equivalences may require
> reloading via the argument pointer. */
>
>
> Jakub
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] EH_USES df fix (PR target/37378, take 2)
2008-10-27 21:14 ` [PATCH] EH_USES df fix (PR target/37378, take 2) Jakub Jelinek
2008-10-27 23:55 ` Kenneth Zadeck
@ 2008-10-28 11:27 ` Eric Botcazou
2008-10-28 11:30 ` Jakub Jelinek
1 sibling, 1 reply; 6+ messages in thread
From: Eric Botcazou @ 2008-10-28 11:27 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches, Kenneth Zadeck, Ian Lance Taylor
> The following alternative patch makes EH_USES registers just live
> everywhere, which IMHO matches best what we really want - let those
> registers stay live through the whole function.
Just to clarify: that goes farther than what was done in flow.c, right?
--
Eric Botcazou
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] EH_USES df fix (PR target/37378, take 2)
2008-10-28 11:27 ` Eric Botcazou
@ 2008-10-28 11:30 ` Jakub Jelinek
2008-11-24 11:15 ` Eric Botcazou
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2008-10-28 11:30 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches, Kenneth Zadeck, Ian Lance Taylor
On Tue, Oct 28, 2008 at 08:20:39AM +0100, Eric Botcazou wrote:
> > The following alternative patch makes EH_USES registers just live
> > everywhere, which IMHO matches best what we really want - let those
> > registers stay live through the whole function.
>
> Just to clarify: that goes farther than what was done in flow.c, right?
Yes, the only real difference should be bb chains ending with
a infinite cycle though.
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] EH_USES df fix (PR target/37378, take 2)
2008-10-28 11:30 ` Jakub Jelinek
@ 2008-11-24 11:15 ` Eric Botcazou
0 siblings, 0 replies; 6+ messages in thread
From: Eric Botcazou @ 2008-11-24 11:15 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches, Kenneth Zadeck, Ian Lance Taylor
[-- Attachment #1: Type: text/plain, Size: 712 bytes --]
> Yes, the only real difference should be bb chains ending with
> a infinite cycle though.
OK, I've installed the first version (the one strictly equivalent to flow.c)
on the 4.3 branch since the problem is responsible for the couple of ACATS
regressions on the IA-64 there:
http://gcc.gnu.org/ml/gcc-testresults/2008-11/msg02040.html
http://gcc.gnu.org/ml/gcc-testresults/2008-11/msg02041.html
Tested on ia64-suse-linux.
2008-11-24 Jakub Jelinek <jakub@redhat.com>
Eric Botcazou <ebotcazou@adacore.com>
* df-scan.c (df_get_call_refs): For unconditional noreturn calls
add EH_USES regs as artificial uses.
(df_get_entry_block_def_set): Don't handle EH_USES here.
--
Eric Botcazou
[-- Attachment #2: p.diff --]
[-- Type: text/x-diff, Size: 1463 bytes --]
Index: df-scan.c
===================================================================
--- df-scan.c (revision 142121)
+++ df-scan.c (working copy)
@@ -3171,7 +3171,22 @@ df_get_call_refs (struct df_collection_r
}
BITMAP_FREE (defs_generated);
- return;
+
+#ifdef EH_USES
+ if ((flags & DF_REF_CONDITIONAL) == 0
+ && find_reg_note (insn, REG_NORETURN, 0))
+ {
+ unsigned int i;
+ /* This code is putting in an artificial ref for the use at the
+ BOTTOM of the block with noreturn call, as EH_USES registers need
+ to be live until epilogue or noreturn call, for debugging purposes
+ as well as any insns that might throw. */
+ for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
+ if (EH_USES (i))
+ df_ref_record (collection_rec, regno_reg_rtx[i],
+ NULL, bb, NULL, DF_REF_REG_USE, 0);
+ }
+#endif
}
/* Collect all refs in the INSN. This function is free of any
@@ -3592,16 +3607,6 @@ df_get_entry_block_def_set (bitmap entry
/* These registers are live everywhere. */
if (!reload_completed)
{
-#ifdef EH_USES
- /* The ia-64, the only machine that uses this, does not define these
- until after reload. */
- for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
- if (EH_USES (i))
- {
- bitmap_set_bit (entry_block_defs, i);
- }
-#endif
-
#if FRAME_POINTER_REGNUM != ARG_POINTER_REGNUM
/* Pseudos with argument area equivalences may require
reloading via the argument pointer. */
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-11-24 8:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-10-27 15:24 [PATCH] EH_USES df fix (PR target/37378) Jakub Jelinek
2008-10-27 21:14 ` [PATCH] EH_USES df fix (PR target/37378, take 2) Jakub Jelinek
2008-10-27 23:55 ` Kenneth Zadeck
2008-10-28 11:27 ` Eric Botcazou
2008-10-28 11:30 ` Jakub Jelinek
2008-11-24 11:15 ` Eric Botcazou
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).