diff mbox

RFA: dead_debug_* ICE

Message ID 87lijd7zmj.fsf@talisman.home
State New
Headers show

Commit Message

Richard Sandiford June 24, 2012, 10:50 a.m. UTC 
gcc.c-torture/compile/vector-2.c fails on mips64-elf with RTL checking
enabled because dead_debug_insert_temp tries to read the REGNO of something
that it has already replaced with a debug temporary.  The original register
was (reg:TI 2), which occupies two hard registers, and so each reference
has two df_refs associated with it.  We replace the use of the register
with a temporary when processing the def/uses pair for register 2,
then consider doing the same thing for register 3.  The "reg" we
read the second time is the debug temporary installed for register 2.
We later to try to read its REGNO.

Since the code is dealing with complete replacements of a use site,
I think it should be using the REGNO of that site rather than the
DF_REF_REGNO.  The code isn't set up to try to do partial replacements
of multi-word hard registers (which would be a bit tricky anyway).

Tested on mips64-elf and x86_64-linux-gnu.  OK to install?

Richard


gcc/
	* df.h (dead_debug_add): Remove third argument.
	* df-problems.c (dead_debug_add): Likewise.  Use the REGNO of the
	REG that we want to replace instead.
	(dead_debug_insert_temp): Use the REGNO of the reg that we want
	to replace instead of DF_REF_REGNO.  Require there to always be
	at least one such use.  Check for cases where the same location
	has more than df_ref associated with it.
	(df_note_bb_compute): Remove third dead_debug_add argument.
	* dce.c (word_dce_process_block): Likewise.

Comments

Alexandre Oliva June 25, 2012, 5:23 a.m. UTC | #1 
On Jun 24, 2012, Richard Sandiford <rdsandiford@googlemail.com> wrote:

> gcc.c-torture/compile/vector-2.c fails on mips64-elf with RTL checking
> enabled because dead_debug_insert_temp tries to read the REGNO of something
> that it has already replaced with a debug temporary.

This sounds like http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53740

The patch looks reasonable to me, but I don't think I'm entitled to
approve it.
Jakub Jelinek June 25, 2012, 5:56 a.m. UTC | #2 
On Sun, Jun 24, 2012 at 11:50:12AM +0100, Richard Sandiford wrote:
> gcc/

Please mention PR debug/53740 here.

> 	* df.h (dead_debug_add): Remove third argument.
> 	* df-problems.c (dead_debug_add): Likewise.  Use the REGNO of the
> 	REG that we want to replace instead.
> 	(dead_debug_insert_temp): Use the REGNO of the reg that we want
> 	to replace instead of DF_REF_REGNO.  Require there to always be
> 	at least one such use.  Check for cases where the same location
> 	has more than df_ref associated with it.
> 	(df_note_bb_compute): Remove third dead_debug_add argument.
> 	* dce.c (word_dce_process_block): Likewise.

Ok, thanks.

	Jakub
Jakub Jelinek June 25, 2012, 1:57 p.m. UTC | #3 
On Sun, Jun 24, 2012 at 11:50:12AM +0100, Richard Sandiford wrote:
> --- gcc/df-problems.c	2012-06-23 13:28:55.576128246 +0100
> +++ gcc/df-problems.c	2012-06-24 11:25:36.851531368 +0100
> @@ -3209,7 +3210,8 @@ dead_debug_insert_temp (struct dead_debu
>       the widest referenced mode.  */
>    while ((cur = *tailp))
>      {
> -      if (DF_REF_REGNO (cur->use) == uregno)
> +      cur_reg = *DF_REF_REAL_LOC (cur->use);
> +      if (REGNO (cur_reg) == uregno)
>  	{
>  	  *usesp = cur;
>  	  usesp = &cur->next;
> @@ -3217,21 +3219,13 @@ dead_debug_insert_temp (struct dead_debu
>  	  cur->next = NULL;
>  	  if (!reg
>  	      || (GET_MODE_BITSIZE (GET_MODE (reg))
> -		  < GET_MODE_BITSIZE (GET_MODE (*DF_REF_REAL_LOC (cur->use)))))
> -	    reg = *DF_REF_REAL_LOC (cur->use);
> +		  < GET_MODE_BITSIZE (GET_MODE (cur_reg))))
> +	    reg = cur_reg;
>  	}
>        else
>  	tailp = &(*tailp)->next;
>      }
>  
> -  /* We may have dangling bits in debug->used for registers that were part
> -     of a multi-register use, one component of which has been reset.  */
> -  if (reg == NULL)
> -    {
> -      gcc_checking_assert (!uses);
> -      return 0;
> -    }
> -
>    gcc_checking_assert (uses);
>  
>    breg = reg;

Unfortunately this seems to break bootstrap on i686-linux.
Delta-reduced testcase is:
typedef _Complex float __attribute__ ((mode (TC))) __complex128;
extern __complex128 clogq (__complex128) __attribute__ ((__nothrow__));
__complex128
cacoshq (__complex128 x)
{
  __complex128 res;
  int rcls = __builtin_fpclassify (0, 1, 4, 3, 2, __real__ x);
  int icls = __builtin_fpclassify (0, 1, 4, 3, 2, __imag__ x);
  if (rcls <= 1 || icls <= 1)
    {
      __complex128 y;
      res = clogq (y);
      if (__real__ res < 0.0)
        res = -res;
    }
  return res;
}
which ICEs at -O2 -g -fPIC -m32 in the above gcc_checking_assert (uses)
now.

	Jakub
Richard Sandiford June 25, 2012, 2:21 p.m. UTC | #4 
Jakub Jelinek <jakub@redhat.com> writes:
> Unfortunately this seems to break bootstrap on i686-linux.
> Delta-reduced testcase is:
> typedef _Complex float __attribute__ ((mode (TC))) __complex128;
> extern __complex128 clogq (__complex128) __attribute__ ((__nothrow__));
> __complex128
> cacoshq (__complex128 x)
> {
>   __complex128 res;
>   int rcls = __builtin_fpclassify (0, 1, 4, 3, 2, __real__ x);
>   int icls = __builtin_fpclassify (0, 1, 4, 3, 2, __imag__ x);
>   if (rcls <= 1 || icls <= 1)
>     {
>       __complex128 y;
>       res = clogq (y);
>       if (__real__ res < 0.0)
>         res = -res;
>     }
>   return res;
> }
> which ICEs at -O2 -g -fPIC -m32 in the above gcc_checking_assert (uses)
> now.

Gah, sorry for the breakage.  I don't have time to look at it right now,
so I've reverted the patch.  I'll leave the proper fix to Alex.

Richard
Alexandre Oliva June 26, 2012, 8:18 p.m. UTC | #5 
On Jun 25, 2012, Richard Sandiford <rdsandiford@googlemail.com> wrote:

> I'll leave the proper fix to Alex.

ACK.  Thanks for the patch, it should help me get started.
diff mbox

Patch

Index: gcc/df.h
===================================================================
--- gcc/df.h	2012-06-23 13:28:55.575128246 +0100
+++ gcc/df.h	2012-06-24 11:25:36.852531367 +0100
@@ -1138,7 +1138,7 @@  enum debug_temp_where
 
 extern void dead_debug_init (struct dead_debug *, bitmap);
 extern void dead_debug_finish (struct dead_debug *, bitmap);
-extern void dead_debug_add (struct dead_debug *, df_ref, unsigned int);
+extern void dead_debug_add (struct dead_debug *, df_ref);
 extern int dead_debug_insert_temp (struct dead_debug *,
 				   unsigned int uregno, rtx insn,
 				   enum debug_temp_where);
Index: gcc/df-problems.c
===================================================================
--- gcc/df-problems.c	2012-06-23 13:28:55.576128246 +0100
+++ gcc/df-problems.c	2012-06-24 11:25:36.851531368 +0100
@@ -3165,10 +3165,10 @@  dead_debug_finish (struct dead_debug *de
     }
 }
 
-/* Add USE to DEBUG.  It must be a dead reference to UREGNO in a debug
+/* Add USE to DEBUG.  It must be a dead reference to a register in a debug
    insn.  Create a bitmap for DEBUG as needed.  */
 void
-dead_debug_add (struct dead_debug *debug, df_ref use, unsigned int uregno)
+dead_debug_add (struct dead_debug *debug, df_ref use)
 {
   struct dead_debug_use *newddu = XNEW (struct dead_debug_use);
 
@@ -3179,7 +3179,7 @@  dead_debug_add (struct dead_debug *debug
   if (!debug->used)
     debug->used = BITMAP_ALLOC (NULL);
 
-  bitmap_set_bit (debug->used, uregno);
+  bitmap_set_bit (debug->used, REGNO (*DF_REF_REAL_LOC (use)));
 }
 
 /* If UREGNO is referenced by any entry in DEBUG, emit a debug insn
@@ -3201,6 +3201,7 @@  dead_debug_insert_temp (struct dead_debu
   rtx breg;
   rtx dval;
   rtx bind;
+  rtx cur_reg;
 
   if (!debug->used || !bitmap_clear_bit (debug->used, uregno))
     return 0;
@@ -3209,7 +3210,8 @@  dead_debug_insert_temp (struct dead_debu
      the widest referenced mode.  */
   while ((cur = *tailp))
     {
-      if (DF_REF_REGNO (cur->use) == uregno)
+      cur_reg = *DF_REF_REAL_LOC (cur->use);
+      if (REGNO (cur_reg) == uregno)
 	{
 	  *usesp = cur;
 	  usesp = &cur->next;
@@ -3217,21 +3219,13 @@  dead_debug_insert_temp (struct dead_debu
 	  cur->next = NULL;
 	  if (!reg
 	      || (GET_MODE_BITSIZE (GET_MODE (reg))
-		  < GET_MODE_BITSIZE (GET_MODE (*DF_REF_REAL_LOC (cur->use)))))
-	    reg = *DF_REF_REAL_LOC (cur->use);
+		  < GET_MODE_BITSIZE (GET_MODE (cur_reg))))
+	    reg = cur_reg;
 	}
       else
 	tailp = &(*tailp)->next;
     }
 
-  /* We may have dangling bits in debug->used for registers that were part
-     of a multi-register use, one component of which has been reset.  */
-  if (reg == NULL)
-    {
-      gcc_checking_assert (!uses);
-      return 0;
-    }
-
   gcc_checking_assert (uses);
 
   breg = reg;
@@ -3340,15 +3334,21 @@  dead_debug_insert_temp (struct dead_debu
   /* Adjust all uses.  */
   while ((cur = uses))
     {
-      if (GET_MODE (*DF_REF_REAL_LOC (cur->use)) == GET_MODE (reg))
-	*DF_REF_REAL_LOC (cur->use) = dval;
-      else
-	*DF_REF_REAL_LOC (cur->use)
-	  = gen_lowpart_SUBREG (GET_MODE (*DF_REF_REAL_LOC (cur->use)), dval);
-      /* ??? Should we simplify subreg of subreg?  */
-      if (debug->to_rescan == NULL)
-	debug->to_rescan = BITMAP_ALLOC (NULL);
-      bitmap_set_bit (debug->to_rescan, INSN_UID (DF_REF_INSN (cur->use)));
+      /* If the reference spans multiple hard registers, we'll have
+	 a use for each one.  Only change each reference once.  */
+      cur_reg = *DF_REF_REAL_LOC (cur->use);
+      if (REG_P (cur_reg))
+	{
+	  if (GET_MODE (cur_reg) == GET_MODE (reg))
+	    *DF_REF_REAL_LOC (cur->use) = dval;
+      	  else
+	    *DF_REF_REAL_LOC (cur->use)
+	      = gen_lowpart_SUBREG (GET_MODE (cur_reg), dval);
+	  /* ??? Should we simplify subreg of subreg?  */
+	  if (debug->to_rescan == NULL)
+	    debug->to_rescan = BITMAP_ALLOC (NULL);
+	  bitmap_set_bit (debug->to_rescan, INSN_UID (DF_REF_INSN (cur->use)));
+        }
       uses = cur->next;
       XDELETE (cur);
     }
@@ -3547,7 +3547,7 @@  df_note_bb_compute (unsigned int bb_inde
 			 debug insns either.  */
 		      if (!bitmap_bit_p (artificial_uses, uregno)
 			  && !df_ignore_stack_reg (uregno))
-			dead_debug_add (&debug, use, uregno);
+			dead_debug_add (&debug, use);
 		      continue;
 		    }
 		  break;
Index: gcc/dce.c
===================================================================
--- gcc/dce.c	2012-06-23 13:28:55.575128246 +0100
+++ gcc/dce.c	2012-06-24 11:25:36.850531369 +0100
@@ -848,7 +848,7 @@  word_dce_process_block (basic_block bb,
 		  == 2 * UNITS_PER_WORD)
 	      && !bitmap_bit_p (local_live, 2 * DF_REF_REGNO (*use_rec))
 	      && !bitmap_bit_p (local_live, 2 * DF_REF_REGNO (*use_rec) + 1))
-	    dead_debug_add (&debug, *use_rec, DF_REF_REGNO (*use_rec));
+	    dead_debug_add (&debug, *use_rec);
       }
     else if (INSN_P (insn))
       {
@@ -938,7 +938,7 @@  dce_process_block (basic_block bb, bool
 	for (use_rec = DF_INSN_USES (insn); *use_rec; use_rec++)
 	  if (!bitmap_bit_p (local_live, DF_REF_REGNO (*use_rec))
 	      && !bitmap_bit_p (au, DF_REF_REGNO (*use_rec)))
-	    dead_debug_add (&debug, *use_rec, DF_REF_REGNO (*use_rec));
+	    dead_debug_add (&debug, *use_rec);
       }
     else if (INSN_P (insn))
       {