This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix -fvar-tracking


Hi!

The following patch attempts to fix some brokeness -fvar-tracking
creates, especially on big endian targets.

E.g.
/* { dg-options "-m32 -O2 -g" } */
extern int m;
struct A { unsigned char a; };
struct B { void *b; };
extern struct B *fn (int x);
struct C { struct C *c; };
static unsigned char *d;
typedef int (*T) (struct C *, void *);
struct C *foo (struct C *, T, void *);
int fx (struct C *, void *);

static struct C *
bar (struct C *x, unsigned char y, unsigned char z)
{
  unsigned char v[2] = { y, z };
  return foo (x, fx, v);
}

struct C *
baz (struct A *x, long long int y)
{
  struct B *b;
  struct C *c;
  long long int z;

  if (!(m == 4 || m == 2))
    return 0;
  z = x->a;
  b = fn (z);
  if (!b)
    return 0;
  c = (struct C *) b->b;
  if (!c)
    return 0;
  if (m == 2 && z >= 240)
    z -= 240;
  else if (d)
    z = d[z];
  if (z == 255)
    return 0;
  return bar (c->c, z, y);
}

emits in one range for the y variable (DImode, i.e. 2 hard registers):
        .4byte  .LVL0-.Ltext0    # Location list begin address (*.LLST2)
        .4byte  .LVL2-.Ltext0    # Location list end address (*.LLST2)
        .2byte  0x6      # Location expression size
        .byte   0x55     # DW_OP_reg5
        .byte   0x93     # DW_OP_piece
        .byte   0x4      # uleb128 0x4
        .byte   0x56     # DW_OP_reg6
        .byte   0x93     # DW_OP_piece
        .byte   0x4      # uleb128 0x4
        .4byte  .LVL2-.Ltext0    # Location list begin address (*.LLST2)
        .4byte  .LVL3-.Ltext0    # Location list end address (*.LLST2)
        .2byte  0xb      # Location expression size
        .byte   0x55     # DW_OP_reg5
        .byte   0x93     # DW_OP_piece
        .byte   0x4      # uleb128 0x4
        .byte   0x56     # DW_OP_reg6
        .byte   0x93     # DW_OP_piece
        .byte   0x4      # uleb128 0x4
        .byte   0x93     # DW_OP_piece
        .byte   0x8      # uleb128 0x8
        .byte   0x56     # DW_OP_reg6
        .byte   0x93     # DW_OP_piece
        .byte   0x4      # uleb128 0x4
...

The first location expression is obviously the right one, for
both ranges and there are multiple bugs in the second location
descriptor.  First of all, eventhough the size is 8 bytes == 2 registers,
it emits something as if it was 12 bytes big, with the last 4 bytes
in the same register as the second 4 bytes.  Plus the adjacent
DW_OP_piece ops can't be right either, DW_OP_piece sets size of the
top of the stack object, which is DW_OP_reg6 in that case, and that
can't have size 4 bytes and 8 bytes at the same time.

The first bug occurs especially, but not limited to, big endian targets.
The problem is that var-tracking.c does not care about register modes almost
at all, so e.g. when it sees
(set (reg:SI n [ foo ]) (something))
and later on
(set (mem:QI xxx) (reg:QI n [ foo+3 ]))
- on big endian lowpart QI subreg of the 32-bit reg has +3 offset - it
records
offset = 0 (reg:SI n)
offset = 3 (reg:QI n)
instead of figuring out that the latter occurence should be just ignored
(perhaps just moved the SI reg to front as most recent use), as it is
only use of a subreg and the full reg covering it is already recorded.
When emit_note_insn_var_location goes to emit the thing as
NOTE_INSN_VAR_LOCATION, it will happily emit (reg:SI n) with DW_OP_piece 4
and then it sees a (reg:QI n), so it emits it with DW_OP_piece 1.

The adjacent DW_OP_piece bug is unrelated, e.g. a DCmode variable
would create it too.  There are several places in dwarf2out.c that emit
a location descriptors and some of them concatenate these with DW_OP_piece.
But if DW_OP_piece is already used in the loc descriptors that are
concatenated, we create the bogus double DW_OP_pieces.

The following patch does not try to teach var-tracking.c internals about
register (and memory) modes (I have started working on it, but it needs
more time), only attempts to limit the damage by outputing only sane
location descriptors.  There are 2 changes in emit_note_insn_var_location,
one is to skip objects at offsets already covered by previous object
(so in the cases above e.g. when there is a DImode object at offset
0 and SImode at offset 4, the latter is skipped) and the other is
an attempt to coalesce 2 adjacent hard registers or memory locations.
Although multi-register hard registers will be splitted again by
multiple_reg_loc_descriptor, it is IMHO still worth doing it, as
otherwise we can end up with too many separate location ranges
with identical location descriptors (e.g. when in one place
a DImode reg is used and in another 2 adjacent SImode regs with
the same starting REGNO).  The dwarf2out.c change checks if the
last op in the location descriptor is DW_OP_piece, if it is,
it assumes it contains several location descriptions contatenaced,
with DW_OP_piece each, and does not add a new DW_OP_piece, otherwise
a DW_OP_piece is added.

So far I have tested it on ppc 32-bit Linux kernel's vmlinux image.
With this patch, the .debug_loc section is around 17KB smaller
(0.5%) as some location ranges that used to be considered separate,
eventhough the variable is provably in the same registers, can now
be merged and some bogus location descriptors corrected.

Ok for HEAD and 4.0?

2005-06-04  Jakub Jelinek  <jakub@redhat.com>

	* dwarf2out.c (add_loc_descr_op_piece): New function.
	(multiple_reg_loc_descriptor, concat_loc_descriptor,
	loc_descriptor): Use it.

	* var-tracking.c: Include regs.h and expr.h.
	(emit_note_insn_var_location): Skip over pieces where offset
	is smaller than previous offset plus previous piece mode size.
	Optimize adjacent hard registers or memory locations.
	* Makefile.in (var-tracking.o): Depend on $(REGS_H) and $(EXPR_H).

--- gcc/dwarf2out.c.jj	2005-05-16 23:31:01.000000000 +0200
+++ gcc/dwarf2out.c	2005-06-03 17:38:55.000000000 +0200
@@ -2605,6 +2605,7 @@ static const char *dwarf_stack_op_name (
 static dw_loc_descr_ref new_loc_descr (enum dwarf_location_atom,
 				       unsigned HOST_WIDE_INT, unsigned HOST_WIDE_INT);
 static void add_loc_descr (dw_loc_descr_ref *, dw_loc_descr_ref);
+static void add_loc_descr_op_piece (dw_loc_descr_ref *, int);
 static unsigned long size_of_loc_descr (dw_loc_descr_ref);
 static unsigned long size_of_locs (dw_loc_descr_ref);
 static void output_loc_operands (dw_loc_descr_ref);
@@ -2957,6 +2958,27 @@ add_loc_descr (dw_loc_descr_ref *list_he
   *d = descr;
 }
 
+
+/* Optionally add a DW_OP_piece term to a location description expression.
+   DW_OP_piece is only added if the location description expression already
+   doesn't end with DW_OP_piece.  */
+
+static void
+add_loc_descr_op_piece (dw_loc_descr_ref *list_head, int size)
+{
+  dw_loc_descr_ref loc;
+
+  if (*list_head != NULL)
+    {
+      /* Find the end of the chain.  */
+      for (loc = *list_head; loc->dw_loc_next != NULL; loc = loc->dw_loc_next)
+	;
+
+      if (loc->dw_loc_opc != DW_OP_piece)
+	loc->dw_loc_next = new_loc_descr (DW_OP_piece, size, 0);
+    }
+}
+
 /* Return the size of a location descriptor.  */
 
 static unsigned long
@@ -8406,7 +8428,7 @@ multiple_reg_loc_descriptor (rtx rtl, rt
 
 	  t = one_reg_loc_descriptor (reg);
 	  add_loc_descr (&loc_result, t);
-	  add_loc_descr (&loc_result, new_loc_descr (DW_OP_piece, size, 0));
+	  add_loc_descr_op_piece (&loc_result, size);
 	  ++reg;
 	}
       return loc_result;
@@ -8426,7 +8448,7 @@ multiple_reg_loc_descriptor (rtx rtl, rt
       t = one_reg_loc_descriptor (REGNO (XVECEXP (regs, 0, i)));
       add_loc_descr (&loc_result, t);
       size = GET_MODE_SIZE (GET_MODE (XVECEXP (regs, 0, 0)));
-      add_loc_descr (&loc_result, new_loc_descr (DW_OP_piece, size, 0));
+      add_loc_descr_op_piece (&loc_result, size);
     }
   return loc_result;
 }
@@ -8729,14 +8751,10 @@ concat_loc_descriptor (rtx x0, rtx x1)
     return 0;
 
   cc_loc_result = x0_ref;
-  add_loc_descr (&cc_loc_result,
-		 new_loc_descr (DW_OP_piece,
-				GET_MODE_SIZE (GET_MODE (x0)), 0));
+  add_loc_descr_op_piece (&cc_loc_result, GET_MODE_SIZE (GET_MODE (x0)));
 
   add_loc_descr (&cc_loc_result, x1_ref);
-  add_loc_descr (&cc_loc_result,
-		 new_loc_descr (DW_OP_piece,
-				GET_MODE_SIZE (GET_MODE (x1)), 0));
+  add_loc_descr_op_piece (&cc_loc_result, GET_MODE_SIZE (GET_MODE (x1)));
 
   return cc_loc_result;
 }
@@ -8801,8 +8819,7 @@ loc_descriptor (rtx rtl, bool can_use_fb
 	loc_result = loc_descriptor (XEXP (RTVEC_ELT (par_elems, 0), 0),
 				     can_use_fbreg);
 	mode = GET_MODE (XEXP (RTVEC_ELT (par_elems, 0), 0));
-	add_loc_descr (&loc_result,
-		       new_loc_descr (DW_OP_piece, GET_MODE_SIZE (mode), 0));
+	add_loc_descr_op_piece (&loc_result, GET_MODE_SIZE (mode));
 	for (i = 1; i < num_elem; i++)
 	  {
 	    dw_loc_descr_ref temp;
@@ -8811,9 +8828,7 @@ loc_descriptor (rtx rtl, bool can_use_fb
 				   can_use_fbreg);
 	    add_loc_descr (&loc_result, temp);
 	    mode = GET_MODE (XEXP (RTVEC_ELT (par_elems, i), 0));
-	    add_loc_descr (&loc_result,
-			   new_loc_descr (DW_OP_piece,
-					  GET_MODE_SIZE (mode), 0));
+	    add_loc_descr_op_piece (&loc_result, GET_MODE_SIZE (mode));
 	  }
       }
       break;
--- gcc/var-tracking.c.jj	2005-01-27 10:28:43.000000000 +0100
+++ gcc/var-tracking.c	2005-06-03 21:53:05.000000000 +0200
@@ -102,6 +102,8 @@
 #include "alloc-pool.h"
 #include "fibheap.h"
 #include "hashtab.h"
+#include "regs.h"
+#include "expr.h"
 
 /* Type of micro operation.  */
 enum micro_operation_type
@@ -2202,10 +2204,12 @@ emit_note_insn_var_location (void **varp
   rtx insn = ((emit_note_data *)data)->insn;
   enum emit_note_where where = ((emit_note_data *)data)->where;
   rtx note;
-  int i;
+  int i, j, n_var_parts;
   bool complete;
   HOST_WIDE_INT last_limit;
   tree type_size_unit;
+  HOST_WIDE_INT offsets[MAX_VAR_PARTS];
+  rtx loc[MAX_VAR_PARTS];
 
 #ifdef ENABLE_CHECKING
   if (!var->decl)
@@ -2214,16 +2218,90 @@ emit_note_insn_var_location (void **varp
 
   complete = true;
   last_limit = 0;
+  n_var_parts = 0;
   for (i = 0; i < var->n_var_parts; i++)
     {
+      enum machine_mode mode, wider_mode;
+
       if (last_limit < var->var_part[i].offset)
 	{
 	  complete = false;
 	  break;
 	}
-      last_limit
-	= (var->var_part[i].offset
-	   + GET_MODE_SIZE (GET_MODE (var->var_part[i].loc_chain->loc)));
+      else if (last_limit > var->var_part[i].offset)
+	continue;
+      offsets[n_var_parts] = var->var_part[i].offset;
+      loc[n_var_parts] = var->var_part[i].loc_chain->loc;
+      mode = GET_MODE (loc[n_var_parts]);
+      last_limit = offsets[n_var_parts] + GET_MODE_SIZE (mode);
+
+      /* Attempt to merge adjacent registers or memory.  */
+      wider_mode = GET_MODE_WIDER_MODE (mode);
+      for (j = i + 1; j < var->n_var_parts; j++)
+	if (last_limit <= var->var_part[j].offset)
+	  break;
+      if (j < var->n_var_parts
+	  && wider_mode != VOIDmode
+	  && GET_CODE (loc[n_var_parts])
+	     == GET_CODE (var->var_part[j].loc_chain->loc)
+	  && mode == GET_MODE (var->var_part[j].loc_chain->loc)
+	  && last_limit == var->var_part[j].offset)
+	{
+	  rtx new_loc = NULL;
+	  rtx loc2 = var->var_part[j].loc_chain->loc;
+
+	  if (REG_P (loc[n_var_parts])
+	      && hard_regno_nregs[REGNO (loc[n_var_parts])][mode] * 2
+		 == hard_regno_nregs[REGNO (loc[n_var_parts])][wider_mode]
+	      && REGNO (loc[n_var_parts])
+		 + hard_regno_nregs[REGNO (loc[n_var_parts])][mode]
+		 == REGNO (loc2))
+	    {
+	      if (! WORDS_BIG_ENDIAN && ! BYTES_BIG_ENDIAN)
+		new_loc = simplify_subreg (wider_mode, loc[n_var_parts],
+					   mode, 0);
+	      else if (WORDS_BIG_ENDIAN && BYTES_BIG_ENDIAN)
+		new_loc = simplify_subreg (wider_mode, loc2, mode, 0);
+	      if (new_loc)
+		{
+		  if (!REG_P (new_loc)
+		      || REGNO (new_loc) != REGNO (loc[n_var_parts]))
+		    new_loc = NULL;
+		  else
+		    REG_ATTRS (new_loc) = REG_ATTRS (loc[n_var_parts]);
+		}
+	    }
+	  else if (MEM_P (loc[n_var_parts])
+		   && GET_CODE (XEXP (loc2, 0)) == PLUS
+		   && GET_CODE (XEXP (XEXP (loc2, 0), 0)) == REG
+		   && GET_CODE (XEXP (XEXP (loc2, 0), 1)) == CONST_INT)
+	    {
+	      if ((GET_CODE (XEXP (loc[n_var_parts], 0)) == REG
+		   && rtx_equal_p (XEXP (loc[n_var_parts], 0),
+				   XEXP (XEXP (loc2, 0), 0))
+		   && INTVAL (XEXP (XEXP (loc2, 0), 1))
+		      == GET_MODE_SIZE (mode))
+		  || (GET_CODE (XEXP (loc[n_var_parts], 0)) == PLUS
+		      && GET_CODE (XEXP (XEXP (loc[n_var_parts], 0), 1))
+			 == CONST_INT
+		      && rtx_equal_p (XEXP (XEXP (loc[n_var_parts], 0), 0),
+				      XEXP (XEXP (loc2, 0), 0))
+		      && INTVAL (XEXP (XEXP (loc[n_var_parts], 0), 1))
+			 + GET_MODE_SIZE (mode)
+			 == INTVAL (XEXP (XEXP (loc2, 0), 1))))
+		new_loc = adjust_address_nv (loc[n_var_parts],
+					     wider_mode, 0);
+	    }
+
+	  if (new_loc)
+	    {
+	      loc[n_var_parts] = new_loc;
+	      mode = wider_mode;
+	      last_limit = offsets[n_var_parts] + GET_MODE_SIZE (mode);
+	      i = j;
+	    }
+	}
+      ++n_var_parts;
     }
   type_size_unit = TYPE_SIZE_UNIT (TREE_TYPE (var->decl));
   if ((unsigned HOST_WIDE_INT) last_limit < TREE_INT_CST_LOW (type_size_unit))
@@ -2239,26 +2317,24 @@ emit_note_insn_var_location (void **varp
       NOTE_VAR_LOCATION (note) = gen_rtx_VAR_LOCATION (VOIDmode, var->decl,
 						       NULL_RTX);
     }
-  else if (var->n_var_parts == 1)
+  else if (n_var_parts == 1)
     {
       rtx expr_list
-	= gen_rtx_EXPR_LIST (VOIDmode,
-			     var->var_part[0].loc_chain->loc,
-			     GEN_INT (var->var_part[0].offset));
+	= gen_rtx_EXPR_LIST (VOIDmode, loc[0], GEN_INT (offsets[0]));
 
       NOTE_VAR_LOCATION (note) = gen_rtx_VAR_LOCATION (VOIDmode, var->decl,
 						       expr_list);
     }
-  else if (var->n_var_parts)
+  else if (n_var_parts)
     {
-      rtx argp[MAX_VAR_PARTS];
       rtx parallel;
 
-      for (i = 0; i < var->n_var_parts; i++)
-	argp[i] = gen_rtx_EXPR_LIST (VOIDmode, var->var_part[i].loc_chain->loc,
-				     GEN_INT (var->var_part[i].offset));
+      for (i = 0; i < n_var_parts; i++)
+	loc[i]
+	  = gen_rtx_EXPR_LIST (VOIDmode, loc[i], GEN_INT (offsets[i]));
+
       parallel = gen_rtx_PARALLEL (VOIDmode,
-				   gen_rtvec_v (var->n_var_parts, argp));
+				   gen_rtvec_v (n_var_parts, loc));
       NOTE_VAR_LOCATION (note) = gen_rtx_VAR_LOCATION (VOIDmode, var->decl,
 						       parallel);
     }
--- gcc/Makefile.in.jj	2005-05-04 01:05:16.000000000 +0200
+++ gcc/Makefile.in	2005-06-03 20:55:06.000000000 +0200
@@ -1989,7 +1989,8 @@ df.o : df.c $(CONFIG_H) $(SYSTEM_H) core
    $(BASIC_BLOCK_H) $(DF_H)
 var-tracking.o : var-tracking.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \
    $(RTL_H) $(TREE_H) hard-reg-set.h insn-config.h reload.h $(FLAGS_H) \
-   $(BASIC_BLOCK_H) output.h sbitmap.h alloc-pool.h $(FIBHEAP_H) $(HASHTAB_H)
+   $(BASIC_BLOCK_H) output.h sbitmap.h alloc-pool.h $(FIBHEAP_H) $(HASHTAB_H) \
+   $(REGS_H) $(EXPR_H)
 conflict.o : conflict.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(OBSTACK_H) \
    $(HASHTAB_H) $(RTL_H) hard-reg-set.h $(BASIC_BLOCK_H)
 profile.o : profile.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(RTL_H) \

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]