diff mbox

RFA: Fix debug-insn sensitivity in RA

Message ID 87fvtgiy55.fsf@talisman.default
State New
Headers show

Commit Message

Richard Sandiford Sept. 7, 2013, 1:01 p.m. UTC 
Jakub Jelinek <jakub@redhat.com> writes:
> On Sat, Sep 07, 2013 at 12:37:14PM +0100, Richard Sandiford wrote:
>> Steven Bosscher <stevenb.gcc@gmail.com> writes:
>> > Can you please add a test case?
>> 
>> What kind of test would you suggest?  Do we have a harness for testing
>> that -O2 and -O2 -g .text output is identical?
>
> No, but we have -fcompare-debug, which is even stronger than that (compares
> final RTL, so can catch even differences that aren't visible in the .s file,
> but still are relevant).  And, the default way of bootstrapping also
> compares .text etc. after stripping, between stage2 (built without debug)
> and stage3 (with debug).

Here's the patch again with an -fcompare-debug test.  I spent a while
trying to whittle it down, but beyond this it gets very sensitive.

OK to install?

Richard


gcc/
	* ira.c (update_equiv_regs): Only call set_paradoxical_subreg
	for non-debug insns.
	* lra.c (new_insn_reg): Take the containing insn as a parameter.
	Only modify lra_reg_info[].biggest_mode if it's non-debug insn.
	(collect_non_operand_hard_regs, add_regs_to_insn_regno_info): Update
	accordingly.

gcc/testsuite/
	* g++.dg/debug/ra1.C: New test.

Comments

Vladimir Makarov Sept. 8, 2013, 3:08 p.m. UTC | #1 
On 13-09-07 9:01 AM, Richard Sandiford wrote:
> Jakub Jelinek <jakub@redhat.com> writes:
>> On Sat, Sep 07, 2013 at 12:37:14PM +0100, Richard Sandiford wrote:
>>> Steven Bosscher <stevenb.gcc@gmail.com> writes:
>>>> Can you please add a test case?
>>> What kind of test would you suggest?  Do we have a harness for testing
>>> that -O2 and -O2 -g .text output is identical?
>> No, but we have -fcompare-debug, which is even stronger than that (compares
>> final RTL, so can catch even differences that aren't visible in the .s file,
>> but still are relevant).  And, the default way of bootstrapping also
>> compares .text etc. after stripping, between stage2 (built without debug)
>> and stage3 (with debug).
> Here's the patch again with an -fcompare-debug test.  I spent a while
> trying to whittle it down, but beyond this it gets very sensitive.
>
> OK to install?
>
Ok.  It looks reasonable for me too.

Thanks for the patch, Richard.
>
> gcc/
> 	* ira.c (update_equiv_regs): Only call set_paradoxical_subreg
> 	for non-debug insns.
> 	* lra.c (new_insn_reg): Take the containing insn as a parameter.
> 	Only modify lra_reg_info[].biggest_mode if it's non-debug insn.
> 	(collect_non_operand_hard_regs, add_regs_to_insn_regno_info): Update
> 	accordingly.
>
> gcc/testsuite/
> 	* g++.dg/debug/ra1.C: New test.
>
>
diff mbox

Patch

Index: gcc/ira.c
===================================================================
--- gcc/ira.c	2013-09-07 13:59:23.049113927 +0100
+++ gcc/ira.c	2013-09-07 13:59:26.362142367 +0100
@@ -2944,11 +2944,8 @@  update_equiv_regs (void)
      prevent access beyond allocated memory for paradoxical memory subreg.  */
   FOR_EACH_BB (bb)
     FOR_BB_INSNS (bb, insn)
-      {
-	if (! INSN_P (insn))
-	  continue;
-	for_each_rtx (&insn, set_paradoxical_subreg, (void *)pdx_subregs);
-      }
+      if (NONDEBUG_INSN_P (insn))
+	for_each_rtx (&insn, set_paradoxical_subreg, (void *) pdx_subregs);
 
   /* Scan the insns and find which registers have equivalences.  Do this
      in a separate scan of the insns because (due to -fcse-follow-jumps)
Index: gcc/lra.c
===================================================================
--- gcc/lra.c	2013-09-07 13:59:23.049113927 +0100
+++ gcc/lra.c	2013-09-07 13:59:26.363142375 +0100
@@ -480,13 +480,13 @@  init_insn_regs (void)
     = create_alloc_pool ("insn regs", sizeof (struct lra_insn_reg), 100);
 }
 
-/* Create LRA insn related info about referenced REGNO with TYPE
-   (in/out/inout), biggest reference mode MODE, flag that it is
+/* Create LRA insn related info about a reference to REGNO in INSN with
+   TYPE (in/out/inout), biggest reference mode MODE, flag that it is
    reference through subreg (SUBREG_P), flag that is early clobbered
    in the insn (EARLY_CLOBBER), and reference to the next insn reg
    info (NEXT).	 */
 static struct lra_insn_reg *
-new_insn_reg (int regno, enum op_type type, enum machine_mode mode,
+new_insn_reg (rtx insn, int regno, enum op_type type, enum machine_mode mode,
 	      bool subreg_p, bool early_clobber, struct lra_insn_reg *next)
 {
   struct lra_insn_reg *ir;
@@ -494,7 +494,8 @@  new_insn_reg (int regno, enum op_type ty
   ir = (struct lra_insn_reg *) pool_alloc (insn_reg_pool);
   ir->type = type;
   ir->biggest_mode = mode;
-  if (GET_MODE_SIZE (mode) > GET_MODE_SIZE (lra_reg_info[regno].biggest_mode))
+  if (GET_MODE_SIZE (mode) > GET_MODE_SIZE (lra_reg_info[regno].biggest_mode)
+      && NONDEBUG_INSN_P (insn))
     lra_reg_info[regno].biggest_mode = mode;
   ir->subreg_p = subreg_p;
   ir->early_clobber = early_clobber;
@@ -976,7 +977,7 @@  collect_non_operand_hard_regs (rtx *x, l
 		     && ! (FIRST_STACK_REG <= regno
 			   && regno <= LAST_STACK_REG));
 #endif
-		list = new_insn_reg (regno, type, mode, subreg_p,
+		list = new_insn_reg (data->insn, regno, type, mode, subreg_p,
 				     early_clobber, list);
 	      }
 	  }
@@ -1575,7 +1576,7 @@  add_regs_to_insn_regno_info (lra_insn_re
       expand_reg_info ();
       if (bitmap_set_bit (&lra_reg_info[regno].insn_bitmap, uid))
 	{
-	  data->regs = new_insn_reg (regno, type, mode, subreg_p,
+	  data->regs = new_insn_reg (data->insn, regno, type, mode, subreg_p,
 				     early_clobber, data->regs);
 	  return;
 	}
@@ -1587,8 +1588,9 @@  add_regs_to_insn_regno_info (lra_insn_re
 		if (curr->subreg_p != subreg_p || curr->biggest_mode != mode)
 		  /* The info can not be integrated into the found
 		     structure.  */
-		  data->regs = new_insn_reg (regno, type, mode, subreg_p,
-					     early_clobber, data->regs);
+		  data->regs = new_insn_reg (data->insn, regno, type, mode,
+					     subreg_p, early_clobber,
+					     data->regs);
 		else
 		  {
 		    if (curr->type != type)
Index: gcc/testsuite/g++.dg/debug/ra1.C
===================================================================
--- /dev/null	2013-09-07 09:29:02.438764162 +0100
+++ gcc/testsuite/g++.dg/debug/ra1.C	2013-09-07 14:00:09.653513821 +0100
@@ -0,0 +1,77 @@ 
+/* { dg-options "-fcompare-debug" } */
+
+enum signop { SIGNED, UNSIGNED };
+enum tree_code { FOO, BAR };
+enum tree_code_class { tcc_type, tcc_other };
+extern enum tree_code_class tree_code_type[];
+
+struct tree_base {
+  enum tree_code code : 16;
+  unsigned unsigned_flag : 1;
+};
+
+struct tree_def {
+  tree_base base;
+  struct {
+    int precision;
+  } type_common;
+};
+
+typedef tree_def *tree;
+
+struct storage_ref
+{
+  storage_ref (const long *, unsigned int, unsigned int);
+
+  const long *val;
+  unsigned int len;
+  unsigned int precision;
+};
+
+inline storage_ref::storage_ref (const long *val_in,
+				 unsigned int len_in,
+				 unsigned int precision_in)
+  : val (val_in), len (len_in), precision (precision_in)
+{
+}
+
+struct hwi_with_prec
+{
+  long val;
+  unsigned int precision;
+  signop sgn;
+};
+
+inline storage_ref
+decompose (long *scratch, unsigned int precision,
+	   const hwi_with_prec &x)
+{
+  scratch[0] = x.val;
+  if (x.sgn == SIGNED || x.val >= 0 || precision <= sizeof (long) * 8)
+    return storage_ref (scratch, 1, precision);
+  scratch[1] = 0;
+  return storage_ref (scratch, 2, precision);
+}
+
+extern void tree_class_check_failed (int) __attribute__ ((__noreturn__));
+
+inline tree
+tree_class_check (tree t, const enum tree_code_class cls, int x)
+{
+  if (tree_code_type[t->base.code] != cls)
+    tree_class_check_failed (x);
+  return t;
+}
+
+tree wide_int_to_tree (tree, const storage_ref &);
+
+tree
+build_int_cstu (tree type, unsigned long val)
+{
+  hwi_with_prec x;
+  x.val = val;
+  x.precision = tree_class_check (type, tcc_type, 1)->type_common.precision;
+  x.sgn = (signop) tree_class_check (type, tcc_type, 2)->base.unsigned_flag;
+  long scratch[2];
+  return wide_int_to_tree (type, decompose (scratch, x.precision, x));
+}