Re: var-tracking wrt. leaf regs on sparc

2013-02-07 Thread Jakub Jelinek 
On Wed, Feb 06, 2013 at 03:18:27PM -0500, David Miller wrote:
 From: Eric Botcazou ebotca...@adacore.com
 Date: Wed, 06 Feb 2013 11:13:30 +0100
 
  I think testing crtl-uses_only_leaf_regs is sufficient here (and
  while you're at it, you could also test the value of
  HAVE_window_save, which can be 0 if -mflat is passed on the SPARC),
  so
  
  #ifdef HAVE_window_save
  if (HAVE_window_save  !crtl-uses_only_leaf_regs)
{
  
}
  #endif
 
 Yes, this works perfectly, Jakub any objections?

Perhaps some progress, but not fully working.  I guess you should start
with deciding when the regs should be remapped.  Consider even
simple testcase like (-O2 -g -dA):

int
foo (int a, int b)
{
  int c = a;
  int d = a + b;
  int e = a + b;
  return e;
}

Before *.vartrack, all debug_insn as well as normal insns refer to
%i0 and %i1, before your patch some NOTE_INSN_VAR_LOCATION were referring
to %o[01] registers, others to %i[01] registers, with your patch all refer
to %i[01] registers.  leaf_renumber_regs isn't performed on notes (so,
neither NOTE_INSN_VAR_LOCATION nor NOTE_INSN_CALL_ARG_LOCATION are
adjusted).  Then supposedly somewhere in dwarf2out we do some adjustment,
but still end up with d/e loclist of:
.LLST2:
.uaxword.LVL0-.Ltext0   ! Location list begin address (*.LLST2)
.uaxword.LVL1-.Ltext0   ! Location list end address (*.LLST2)
.uahalf 0x6 ! Location expression size
.byte   0x88! DW_OP_breg24
.byte   0   ! sleb128 0
.byte   0x89! DW_OP_breg25
.byte   0   ! sleb128 0
.byte   0x22! DW_OP_plus
.byte   0x9f! DW_OP_stack_value
.uaxword.LVL1-.Ltext0   ! Location list begin address (*.LLST2)
.uaxword.LFE0-.Ltext0   ! Location list end address (*.LLST2)
.uahalf 0x1 ! Location expression size
.byte   0x58! DW_OP_reg8
.uaxword0   ! Location list terminator begin (*.LLST2)
.uaxword0   ! Location list terminator end (*.LLST2)
where I'd expect breg8/breg9 instead.

Jakub

Re: var-tracking wrt. leaf regs on sparc

2013-02-07 Thread David Miller 
From: Jakub Jelinek ja...@redhat.com
Date: Thu, 7 Feb 2013 18:22:32 +0100

 Then supposedly somewhere in dwarf2out we do some adjustment,
 but still end up with d/e loclist of:
 .LLST2:
 .uaxword.LVL0-.Ltext0   ! Location list begin address 
 (*.LLST2)
 .uaxword.LVL1-.Ltext0   ! Location list end address (*.LLST2)
 .uahalf 0x6 ! Location expression size
 .byte   0x88! DW_OP_breg24
 .byte   0   ! sleb128 0
 .byte   0x89! DW_OP_breg25
 .byte   0   ! sleb128 0
 .byte   0x22! DW_OP_plus
 .byte   0x9f! DW_OP_stack_value
 .uaxword.LVL1-.Ltext0   ! Location list begin address 
 (*.LLST2)
 .uaxword.LFE0-.Ltext0   ! Location list end address (*.LLST2)
 .uahalf 0x1 ! Location expression size
 .byte   0x58! DW_OP_reg8
 .uaxword0   ! Location list terminator begin (*.LLST2)
 .uaxword0   ! Location list terminator end (*.LLST2)
 where I'd expect breg8/breg9 instead.

The fix for this is trivial, just a missing leaf renumbering in dwarf2out.c:

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 06cfb18..765d5c5 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -10864,7 +10864,16 @@ based_loc_descr (rtx reg, HOST_WIDE_INT offset,
}
 }
 
-  regno = DWARF_FRAME_REGNUM (REGNO (reg));
+  regno = REGNO (reg);
+#ifdef LEAF_REG_REMAP
+  if (crtl-uses_only_leaf_regs)
+{
+  int leaf_reg = LEAF_REG_REMAP (regno);
+  if (leaf_reg != -1)
+   regno = (unsigned) leaf_reg;
+}
+#endif
+  regno = DWARF_FRAME_REGNUM (regno);
 
   if (!optimize  fde
(fde-drap_reg == regno || fde-vdrap_reg == regno))

Re: var-tracking wrt. leaf regs on sparc

2013-02-07 Thread Jakub Jelinek 
On Thu, Feb 07, 2013 at 02:38:18PM -0500, David Miller wrote:
 The fix for this is trivial, just a missing leaf renumbering in dwarf2out.c:
 
 diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
 index 06cfb18..765d5c5 100644
 --- a/gcc/dwarf2out.c
 +++ b/gcc/dwarf2out.c
 @@ -10864,7 +10864,16 @@ based_loc_descr (rtx reg, HOST_WIDE_INT offset,
   }
  }
  
 -  regno = DWARF_FRAME_REGNUM (REGNO (reg));
 +  regno = REGNO (reg);
 +#ifdef LEAF_REG_REMAP
 +  if (crtl-uses_only_leaf_regs)
 +{
 +  int leaf_reg = LEAF_REG_REMAP (regno);
 +  if (leaf_reg != -1)
 + regno = (unsigned) leaf_reg;
 +}
 +#endif
 +  regno = DWARF_FRAME_REGNUM (regno);
  
if (!optimize  fde
 (fde-drap_reg == regno || fde-vdrap_reg == regno))

This and earlier patch are ok, if it bootstraps/regtests fine, and suitable
ChangeLog entry is provided.
Running gdb testsuite before and after wouldn't hurt though.

Jakub

Re: var-tracking wrt. leaf regs on sparc

2013-02-07 Thread David Miller 
From: David Miller da...@davemloft.net
Date: Thu, 07 Feb 2013 14:38:18 -0500 (EST)

 From: Jakub Jelinek ja...@redhat.com
 Date: Thu, 7 Feb 2013 18:22:32 +0100
 
 Then supposedly somewhere in dwarf2out we do some adjustment,
 but still end up with d/e loclist of:
 .LLST2:
 .uaxword.LVL0-.Ltext0   ! Location list begin address 
 (*.LLST2)
 .uaxword.LVL1-.Ltext0   ! Location list end address (*.LLST2)
 .uahalf 0x6 ! Location expression size
 .byte   0x88! DW_OP_breg24
 .byte   0   ! sleb128 0
 .byte   0x89! DW_OP_breg25
 .byte   0   ! sleb128 0
 .byte   0x22! DW_OP_plus
 .byte   0x9f! DW_OP_stack_value
 .uaxword.LVL1-.Ltext0   ! Location list begin address 
 (*.LLST2)
 .uaxword.LFE0-.Ltext0   ! Location list end address (*.LLST2)
 .uahalf 0x1 ! Location expression size
 .byte   0x58! DW_OP_reg8
 .uaxword0   ! Location list terminator begin (*.LLST2)
 .uaxword0   ! Location list terminator end (*.LLST2)
 where I'd expect breg8/breg9 instead.
 
 The fix for this is trivial, just a missing leaf renumbering in dwarf2out.c:

So the combined patch is below, any objections?

Here is the testsuite diff:

@@ -155,8 +148,8 @@ FAIL: gcc.dg/guality/vla-2.c  -O2 -flto

=== gcc Summary ===

-# of expected passes   2128
-# of unexpected failures   122
+# of expected passes   2135
+# of unexpected failures   115
 # of unexpected successes  31
 # of expected failures 17
 # of unsupported tests 136

This is undoubtedly an improvement.

gcc/

2013-02-07  David S. Miller  da...@davemloft.net

* dwarf2out.c (based_loc_descr): Perform leaf register remapping
on 'reg'.
* var-tracking.c (vt_add_function_parameter): Test the presence of
HAVE_window_save properly and do not remap argument registers when
we have a leaf function.

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 06cfb18..765d5c5 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -10864,7 +10864,16 @@ based_loc_descr (rtx reg, HOST_WIDE_INT offset,
}
 }
 
-  regno = DWARF_FRAME_REGNUM (REGNO (reg));
+  regno = REGNO (reg);
+#ifdef LEAF_REG_REMAP
+  if (crtl-uses_only_leaf_regs)
+{
+  int leaf_reg = LEAF_REG_REMAP (regno);
+  if (leaf_reg != -1)
+   regno = (unsigned) leaf_reg;
+}
+#endif
+  regno = DWARF_FRAME_REGNUM (regno);
 
   if (!optimize  fde
(fde-drap_reg == regno || fde-vdrap_reg == regno))
diff --git a/gcc/var-tracking.c b/gcc/var-tracking.c
index 714acb69..0db1562 100644
--- a/gcc/var-tracking.c
+++ b/gcc/var-tracking.c
@@ -9502,31 +9502,34 @@ vt_add_function_parameter (tree parm)
   /* DECL_INCOMING_RTL uses the INCOMING_REGNO of parameter registers.
  If the target machine has an explicit window save instruction, the
  actual entry value is the corresponding OUTGOING_REGNO instead.  */
-  if (REG_P (incoming)
-   HARD_REGISTER_P (incoming)
-   OUTGOING_REGNO (REGNO (incoming)) != REGNO (incoming))
+  if (HAVE_window_save  !crtl-uses_only_leaf_regs)
 {
-  parm_reg_t p;
-  p.incoming = incoming;
-  incoming
-   = gen_rtx_REG_offset (incoming, GET_MODE (incoming),
- OUTGOING_REGNO (REGNO (incoming)), 0);
-  p.outgoing = incoming;
-  vec_safe_push (windowed_parm_regs, p);
-}
-  else if (MEM_P (incoming)
-   REG_P (XEXP (incoming, 0))
-   HARD_REGISTER_P (XEXP (incoming, 0)))
-{
-  rtx reg = XEXP (incoming, 0);
-  if (OUTGOING_REGNO (REGNO (reg)) != REGNO (reg))
+  if (REG_P (incoming)
+  HARD_REGISTER_P (incoming)
+  OUTGOING_REGNO (REGNO (incoming)) != REGNO (incoming))
{
  parm_reg_t p;
- p.incoming = reg;
- reg = gen_raw_REG (GET_MODE (reg), OUTGOING_REGNO (REGNO (reg)));
- p.outgoing = reg;
+ p.incoming = incoming;
+ incoming
+   = gen_rtx_REG_offset (incoming, GET_MODE (incoming),
+ OUTGOING_REGNO (REGNO (incoming)), 0);
+ p.outgoing = incoming;
  vec_safe_push (windowed_parm_regs, p);
- incoming = replace_equiv_address_nv (incoming, reg);
+   }
+  else if (MEM_P (incoming)
+   REG_P (XEXP (incoming, 0))
+   HARD_REGISTER_P (XEXP (incoming, 0)))
+   {
+ rtx reg = XEXP (incoming, 0);
+ if (OUTGOING_REGNO (REGNO (reg)) != REGNO (reg))
+   {
+ parm_reg_t p;
+ p.incoming = reg;
+ reg = gen_raw_REG (GET_MODE (reg), OUTGOING_REGNO (REGNO (reg)));
+ p.outgoing = reg;
+ vec_safe_push (windowed_parm_regs, p);
+ incoming = replace_equiv_address_nv (incoming, reg);
+   }
}
 }
 #endif

Re: var-tracking wrt. leaf regs on sparc

2013-02-07 Thread David Miller 
From: Jakub Jelinek ja...@redhat.com
Date: Thu, 7 Feb 2013 20:43:32 +0100

 This and earlier patch are ok, if it bootstraps/regtests fine, and suitable
 ChangeLog entry is provided.
 Running gdb testsuite before and after wouldn't hurt though.

I've done all of this, and committed to trunk and the gcc-4.7 branch,
thanks.

In looking at the remaining failures, several have to do with
an early clobber if the first incoming argument register.

The issue is that this is where return values are placed, so we run
into a situation where that incoming argument value can't be
reconstituted in any way by the variable tracking code and thus gdb
says that it has been optimized out.

Many non-x86 cpus are going to run into this problem.  For example,
from pr36728-1.c:

foo:
save%sp, -96, %sp
add %sp, -40, %sp
mov 2, %g2
add %sp, 123, %g1
mov 25, %g4
and %g1, -32, %g1
sethi   %hi(b), %g3
st  %g2, [%g1]
ld  [%fp+92], %g2
nop
ld  [%g1], %i0
add %g2, 14, %g2
and %g2, -8, %g2
sub %sp, %g2, %sp
stb %g4, [%sp+96]
add %sp, 96, %g2
sethi   %hi(a), %g4
nop
return  %i7+8
 nop

Here %i0 is written early, and then the tests can't view 'arg1'
properly later in the function.

Also, I noticed that calculation of the on-stack address of values
with alignment regressed in gcc-4.8 vs. gcc-4.7 Again, in pr36728-1.c,
'y' can be printed properly in gcc-4.7 but in gcc-4.8 it cannot.

I think it might be getting the base register wrong, I'll look
more deeply if I get a chance.

Re: var-tracking wrt. leaf regs on sparc

2013-02-06 Thread Eric Botcazou 
 Now that I understand fully what we're trying to accomplish with the
 DT_OP_GNU_entry_value and DT_OP_GNU_call_site_parameter extensions, it
 does in fact seem like we will need to do leaf register remapping in
 var-tracking.c
 
 Here below is a patch I'm playing with.  It's a rough draft but it
 definitely fixes the pr54200.c problem completely.

Thanks for tackling this.  I cannot really comment on the patch itself (Jakub 
is the resident expert here), only on its idea.  I think that the most correct 
approach would indeed be to also do leaf register remapping in var-tracking.c,
before analyzing the RTL stream, so that everything is correctly exposed.  In 
practice, that might be a little heavy-handed though, so...

 Another way to do this would be to not translate the incoming
 parameter registers (leave them at %i*) if we don't see the window
 save.  That way we only have to play the regno remapping game for
 these specific incoming argument pieces, rather than for everything we
 look at in the RTL stream.

...yes, probably much lighter.  I think testing crtl-uses_only_leaf_regs is 
sufficient here (and while you're at it, you could also test the value of 
HAVE_window_save, which can be 0 if -mflat is passed on the SPARC), so

#ifdef HAVE_window_save
if (HAVE_window_save  !crtl-uses_only_leaf_regs)
  {

  }
#endif

-- 
Eric Botcazou

Re: var-tracking wrt. leaf regs on sparc

2013-02-06 Thread David Miller 
From: Eric Botcazou ebotca...@adacore.com
Date: Wed, 06 Feb 2013 11:13:30 +0100

 I think testing crtl-uses_only_leaf_regs is sufficient here (and
 while you're at it, you could also test the value of
 HAVE_window_save, which can be 0 if -mflat is passed on the SPARC),
 so
 
 #ifdef HAVE_window_save
 if (HAVE_window_save  !crtl-uses_only_leaf_regs)
   {
 
   }
 #endif

Yes, this works perfectly, Jakub any objections?

gcc/

2013-02-06  David S. Miller  da...@davemloft.net

* var-tracking.c (vt_add_function_parameter): Test the presence of
HAVE_window_save properly and do not remap argument registers when
we have a leaf function.

diff --git a/gcc/var-tracking.c b/gcc/var-tracking.c
index 714acb69..0db1562 100644
--- a/gcc/var-tracking.c
+++ b/gcc/var-tracking.c
@@ -9502,31 +9502,34 @@ vt_add_function_parameter (tree parm)
   /* DECL_INCOMING_RTL uses the INCOMING_REGNO of parameter registers.
  If the target machine has an explicit window save instruction, the
  actual entry value is the corresponding OUTGOING_REGNO instead.  */
-  if (REG_P (incoming)
-   HARD_REGISTER_P (incoming)
-   OUTGOING_REGNO (REGNO (incoming)) != REGNO (incoming))
+  if (HAVE_window_save  !crtl-uses_only_leaf_regs)
 {
-  parm_reg_t p;
-  p.incoming = incoming;
-  incoming
-   = gen_rtx_REG_offset (incoming, GET_MODE (incoming),
- OUTGOING_REGNO (REGNO (incoming)), 0);
-  p.outgoing = incoming;
-  vec_safe_push (windowed_parm_regs, p);
-}
-  else if (MEM_P (incoming)
-   REG_P (XEXP (incoming, 0))
-   HARD_REGISTER_P (XEXP (incoming, 0)))
-{
-  rtx reg = XEXP (incoming, 0);
-  if (OUTGOING_REGNO (REGNO (reg)) != REGNO (reg))
+  if (REG_P (incoming)
+  HARD_REGISTER_P (incoming)
+  OUTGOING_REGNO (REGNO (incoming)) != REGNO (incoming))
{
  parm_reg_t p;
- p.incoming = reg;
- reg = gen_raw_REG (GET_MODE (reg), OUTGOING_REGNO (REGNO (reg)));
- p.outgoing = reg;
+ p.incoming = incoming;
+ incoming
+   = gen_rtx_REG_offset (incoming, GET_MODE (incoming),
+ OUTGOING_REGNO (REGNO (incoming)), 0);
+ p.outgoing = incoming;
  vec_safe_push (windowed_parm_regs, p);
- incoming = replace_equiv_address_nv (incoming, reg);
+   }
+  else if (MEM_P (incoming)
+   REG_P (XEXP (incoming, 0))
+   HARD_REGISTER_P (XEXP (incoming, 0)))
+   {
+ rtx reg = XEXP (incoming, 0);
+ if (OUTGOING_REGNO (REGNO (reg)) != REGNO (reg))
+   {
+ parm_reg_t p;
+ p.incoming = reg;
+ reg = gen_raw_REG (GET_MODE (reg), OUTGOING_REGNO (REGNO (reg)));
+ p.outgoing = reg;
+ vec_safe_push (windowed_parm_regs, p);
+ incoming = replace_equiv_address_nv (incoming, reg);
+   }
}
 }
 #endif

Re: var-tracking wrt. leaf regs on sparc

2013-02-06 Thread Eric Botcazou 
 Yes, this works perfectly, Jakub any objections?
 
 gcc/
 
 2013-02-06  David S. Miller  da...@davemloft.net
 
   * var-tracking.c (vt_add_function_parameter): Test the presence of
   HAVE_window_save properly and do not remap argument registers when
   we have a leaf function.

Please put it on the 4.7 branch as well if approved, this worked (by chance) 
before my change.  TIA.

-- 
Eric Botcazou

Re: var-tracking wrt. leaf regs on sparc

2013-02-05 Thread David Miller 
From: David Miller da...@davemloft.net
Date: Tue, 05 Feb 2013 18:18:39 -0500 (EST)

 The only other alternative I can see would be to get everything in
 var-tracking.c and the other subsystems it uses to do leaf register
 remapping, but that seems completely like the wrong way to handle
 this.

Following up to myself... :-)

Now that I understand fully what we're trying to accomplish with the
DT_OP_GNU_entry_value and DT_OP_GNU_call_site_parameter extensions, it
does in fact seem like we will need to do leaf register remapping in
var-tracking.c

Here below is a patch I'm playing with.  It's a rough draft but it
definitely fixes the pr54200.c problem completely.

Another way to do this would be to not translate the incoming
parameter registers (leave them at %i*) if we don't see the window
save.  That way we only have to play the regno remapping game for
these specific incoming argument pieces, rather than for everything we
look at in the RTL stream.

diff --git a/gcc/var-tracking.c b/gcc/var-tracking.c
index 714acb69..14635b9 100644
--- a/gcc/var-tracking.c
+++ b/gcc/var-tracking.c
@@ -1057,6 +1057,32 @@ adjust_mem_stores (rtx loc, const_rtx expr, void *data)
 }
 }
 
+/* Given a regno from the RTL instruction stream, return the
+   actual register number that will be used by final and debug
+   info emission.  */
+static unsigned int
+real_regno (unsigned int regno)
+{
+#ifdef LEAF_REG_REMAP
+  if (regno  FIRST_PSEUDO_REGISTER
+   crtl-uses_only_leaf_regs)
+{
+  int remapped = LEAF_REG_REMAP (regno);
+
+  if (remapped = 0)
+   regno = (unsigned int) remapped;
+}
+#endif
+
+  return regno;
+}
+
+static unsigned int
+real_regno_rtx (rtx reg)
+{
+  return real_regno (REGNO (reg));
+}
+
 /* Simplify INSN.  Remove all {PRE,POST}_{INC,DEC,MODIFY} rtxes,
replace them with their value in the insn and add the side-effects
as other sets to the insn.  */
@@ -1804,12 +1830,12 @@ var_reg_decl_set (dataflow_set *set, rtx loc, enum 
var_init_status initialized,
   if (decl_p)
 dv = dv_from_decl (var_debug_decl (dv_as_decl (dv)));
 
-  for (node = set-regs[REGNO (loc)]; node; node = node-next)
+  for (node = set-regs[real_regno_rtx (loc)]; node; node = node-next)
 if (dv_as_opaque (node-dv) == dv_as_opaque (dv)
 node-offset == offset)
   break;
   if (!node)
-attrs_list_insert (set-regs[REGNO (loc)], dv, offset, loc);
+attrs_list_insert (set-regs[real_regno_rtx (loc)], dv, offset, loc);
   set_variable_part (set, loc, dv, offset, initialized, set_src, iopt);
 }
 
@@ -1875,7 +1901,7 @@ var_reg_delete_and_set (dataflow_set *set, rtx loc, bool 
modify,
   if (initialized == VAR_INIT_STATUS_UNKNOWN)
 initialized = get_init_value (set, loc, dv_from_decl (decl));
 
-  nextp = set-regs[REGNO (loc)];
+  nextp = set-regs[real_regno_rtx (loc)];
   for (node = *nextp; node; node = next)
 {
   next = node-next;
@@ -1904,7 +1930,7 @@ var_reg_delete_and_set (dataflow_set *set, rtx loc, bool 
modify,
 static void
 var_reg_delete (dataflow_set *set, rtx loc, bool clobber)
 {
-  attrs *nextp = set-regs[REGNO (loc)];
+  attrs *nextp = set-regs[real_regno_rtx (loc)];
   attrs node, next;
 
   if (clobber)
@@ -2386,7 +2412,7 @@ val_bind (dataflow_set *set, rtx val, rtx loc, bool 
modified)
   if (REG_P (loc))
 {
   if (modified)
-   var_regno_delete (set, REGNO (loc));
+   var_regno_delete (set, real_regno_rtx (loc));
   var_reg_decl_set (set, loc, VAR_INIT_STATUS_INITIALIZED,
dv_from_value (val), 0, NULL_RTX, INSERT);
 }
@@ -2584,7 +2610,7 @@ val_resolve (dataflow_set *set, rtx val, rtx loc, rtx 
insn)
 {
   attrs node, found = NULL;
 
-  for (node = set-regs[REGNO (loc)]; node; node = node-next)
+  for (node = set-regs[real_regno_rtx (loc)]; node; node = node-next)
if (dv_is_value_p (node-dv)
 GET_MODE (dv_as_value (node-dv)) == GET_MODE (loc))
  {
@@ -2838,7 +2864,8 @@ variable_union (variable src, dataflow_set *set)
{
  if (!((REG_P (node2-loc)
  REG_P (node-loc)
- REGNO (node2-loc) == REGNO (node-loc))
+ (real_regno_rtx (node2-loc)
+== real_regno_rtx (node-loc)))
|| rtx_equal_p (node2-loc, node-loc)))
{
  if (node2-init  node-init)
@@ -2871,7 +2898,8 @@ variable_union (variable src, dataflow_set *set)
  for (node = src-var_part[i].loc_chain; node; node = node-next)
if (!((REG_P (dstnode-loc)
REG_P (node-loc)
-   REGNO (dstnode-loc) == REGNO (node-loc))
+   (real_regno_rtx (dstnode-loc)
+  == real_regno_rtx (node-loc)))
  || rtx_equal_p (dstnode-loc, node-loc)))
  {
location_chain new_node;
@@ -2920,7 +2948,8