Re: [patch] PR debug/53948
On Wed, Jul 18, 2012 at 7:46 PM, Steven Bosscher wrote:
> Hello,
>
> This is my proposed fix for PR53948. We don't want to put user
> variables in callee-clobbered registers, but obviously function
> arguments are OK there. REG_USERVAR_P is set on PARM_DECLs and on user
> variables, so it can't be used to distinguish between the two.
>
> As it turns out, I can hi-jack a bit for that: 'unchanging' (currently
> incorrectly documented as used on REG) for a new macro
> REG_FUNCTION_PARM_P. I found one obvious place where this bit can be
> used instead of REG_USERVAR_P, and probably there are a few more
> places where this is useful (TBD, I'm going to look at all places
> where RTL code looks at tree's PARM_DECL later).
>
> Bootstrapped&tested on powerpc64-unknown-linux-gnu.
> OK for trunk?
Just being pointed back to this patch ... I wonder if simply looking
at REG_EXPR of a REG_USERVAR_P reg and checking whether
it's a PARM_DECL isn't good enough (and simpler)?
I suppose the optabs.c change was the "real" fix, thus sth like
Index: gcc/optabs.c
===
--- gcc/optabs.c(revision 194552)
+++ gcc/optabs.c(working copy)
@@ -3848,9 +3848,13 @@ emit_libcall_block_1 (rtx insns, rtx tar
rtx final_dest = target;
rtx next, last, insn;
- /* If this is a reg with REG_USERVAR_P set, then it could possibly turn
- into a MEM later. Protect the libcall block from this change. */
- if (! REG_P (target) || REG_USERVAR_P (target))
+ /* If this is a parameter with REG_USERVAR_P set, then it could possibly turn
+ into a MEM later (e.g. if a REG_EQUIV note is attached to the insns that
+ sets the reg). Protect the libcall block from this change. */
+ if (! REG_P (target)
+ || (REG_USERVAR_P (target)
+ && REG_EXPR (target) != NULL_TREE
+ && TREE_CODE (REG_EXPR (target)) == PARM_DECL))
target = gen_reg_rtx (GET_MODE (target));
/* If we're using non-call exceptions, a libcall corresponding to an
(untested)
Thanks,
Richard.
> Ciao!
> Steven
>
> PR debug/53948
> * rtl.h (REG_FUNCTION_PARM_P): New flag on a REG. Re-use 'unchaning'.
> * emit-rtl.c (mark_function_parm_reg): New function.
> * function.c (assign_parm_setup_reg): Use mark_function_parm_reg
> instead of mark_user_reg.
> * combine.c (can_change_dest_mode): Preserve REG_FUNCTION_PARM_P.
> * web.c (entry_register): Likewise.
> * reload1.c (reload): Likewise.
> * ira-emit.c (ira_create_new_reg): Likewise.
> * reginfo.c (reg_scan_mark_refs): Likewise.
> * optabs.c (emit_libcall_block_1): Use REG_FUNCTION_PARM_P instead
> of REG_USERVAR_P.
> * regstat.c (dump_reg_info): Print REG_FUNCTION_PARM_P.
> * doc/rtl.texi (REG_FUNCTION_PARM_P): Document it.
> ('unchanging' flag): Fix documentation.
Re: [patch] PR debug/53948
On Wed, Jul 18, 2012 at 08:55:17PM +0200, Jan Kratochvil wrote:
> On Wed, 18 Jul 2012 20:05:46 +0200, Steven Bosscher wrote:
> > I wouldn't know what to test for. Looking for a .loc marker seems a bit
> > fragile.
>
> What is fragile on
> // { dg-final { scan-assembler-times "\\.loc\t1 3 0\\r\\n\t\[^.\]" 6 } }
>
> or something like that. Line numbers are constant for the testcase.
Not all assemblers support .loc markers, sometimes .debug_line is emitted by
gcc directly, and not all targets use dwarf. For the latter we can just put
the test into gcc.dg/debug/dwarf/, for the former we would need a new
dejagnu test. But more importantly, on different arches I'd guess the
number of .loc times might be different.
Better than that would be a guality testcase limited to -O0.
Jakub
Re: [patch] PR debug/53948
On Wed, 18 Jul 2012 20:05:46 +0200, Steven Bosscher wrote:
> I wouldn't know what to test for. Looking for a .loc marker seems a bit
> fragile.
What is fragile on
// { dg-final { scan-assembler-times "\\.loc\t1 3 0\\r\\n\t\[^.\]" 6 } }
or something like that. Line numbers are constant for the testcase.
Thanks,
Jan
Re: [patch] PR debug/53948
On Wed, Jul 18, 2012 at 7:55 PM, Jan Kratochvil wrote: > Hello Steven, > > On Wed, 18 Jul 2012 19:46:16 +0200, Steven Bosscher wrote: >> This is my proposed fix for PR53948. > > I can't speak for the GCC code but could it have a GCC testcase? I wouldn't know what to test for. Looking for a .loc marker seems a bit fragile. Ciao! Steven
Re: [patch] PR debug/53948
Hello Steven, On Wed, 18 Jul 2012 19:46:16 +0200, Steven Bosscher wrote: > This is my proposed fix for PR53948. I can't speak for the GCC code but could it have a GCC testcase? Thanks, Jan
[patch] PR debug/53948
Hello,
This is my proposed fix for PR53948. We don't want to put user
variables in callee-clobbered registers, but obviously function
arguments are OK there. REG_USERVAR_P is set on PARM_DECLs and on user
variables, so it can't be used to distinguish between the two.
As it turns out, I can hi-jack a bit for that: 'unchanging' (currently
incorrectly documented as used on REG) for a new macro
REG_FUNCTION_PARM_P. I found one obvious place where this bit can be
used instead of REG_USERVAR_P, and probably there are a few more
places where this is useful (TBD, I'm going to look at all places
where RTL code looks at tree's PARM_DECL later).
Bootstrapped&tested on powerpc64-unknown-linux-gnu.
OK for trunk?
Ciao!
Steven
PR debug/53948
* rtl.h (REG_FUNCTION_PARM_P): New flag on a REG. Re-use 'unchaning'.
* emit-rtl.c (mark_function_parm_reg): New function.
* function.c (assign_parm_setup_reg): Use mark_function_parm_reg
instead of mark_user_reg.
* combine.c (can_change_dest_mode): Preserve REG_FUNCTION_PARM_P.
* web.c (entry_register): Likewise.
* reload1.c (reload): Likewise.
* ira-emit.c (ira_create_new_reg): Likewise.
* reginfo.c (reg_scan_mark_refs): Likewise.
* optabs.c (emit_libcall_block_1): Use REG_FUNCTION_PARM_P instead
of REG_USERVAR_P.
* regstat.c (dump_reg_info): Print REG_FUNCTION_PARM_P.
* doc/rtl.texi (REG_FUNCTION_PARM_P): Document it.
('unchanging' flag): Fix documentation.
PR53948.diff
Description: Binary data
