Patchwork [mips,debug] Fix PR 54061, mips compiler aborts in testsuite

login
register
mail settings
Submitter Steve Ellcey
Date Dec. 7, 2012, 11:17 p.m.
Message ID <1354922228.4464.88.camel@ubuntu-sellcey>
Download mbox | patch
Permalink /patch/204619/
State New
Headers show

Comments

Steve Ellcey - Dec. 7, 2012, 11:17 p.m.
> On Fri, 2012-12-07 at 14:43 -0800, H.J. Lu wrote:
>
>> > I think you need a way to tell a bad register from a good register
>> > which doesn't have DWARF register number.

Here is a patch using your IGNORED_DWARF_REGNUM idea.

Steve Ellcey
sellcey@mips.com


2012-12-07  Steve Ellcey  <sellcey@mips.com>

	PR target/54061
	* rtl.h (IGNORED_DWARF_REGNUM): New.
	* dwarfwout.c (reg_loc_descriptor): Check for IGNORED_DWARF_REGNUM.
	(mem_loc_descriptor): Ditto.
	* config/mips/mips.c (mips_option_override): Set mips_dbx_regno
	coprocessor entries to IGNORED_DWARF_REGNUM.
Cary Coutant - Dec. 8, 2012, 12:11 a.m.
> 2012-12-07  Steve Ellcey  <sellcey@mips.com>
>
>         PR target/54061
>         * rtl.h (IGNORED_DWARF_REGNUM): New.
>         * dwarfwout.c (reg_loc_descriptor): Check for IGNORED_DWARF_REGNUM.

Typo: s/dwarfwout/dwarf2out/

>         (mem_loc_descriptor): Ditto.
>         * config/mips/mips.c (mips_option_override): Set mips_dbx_regno
>         coprocessor entries to IGNORED_DWARF_REGNUM.

-cary
Richard Sandiford - Dec. 8, 2012, 10:14 a.m.
Steve Ellcey <sellcey@mips.com> writes:
> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
> index b6a2290..bc99f29 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> @@ -16757,6 +16757,9 @@ mips_option_override (void)
>    for (i = FP_REG_FIRST; i <= FP_REG_LAST; i++)
>      mips_dbx_regno[i] = i + start;
>  
> +  for (i = COP0_REG_FIRST; i <= COP3_REG_LAST; i++)
> +    mips_dbx_regno[i] = IGNORED_DWARF_REGNUM;
> +
>    /* Accumulator debug registers use big-endian ordering.  */
>    mips_dbx_regno[HI_REGNUM] = MD_DBX_FIRST + 0;
>    mips_dbx_regno[LO_REGNUM] = MD_DBX_FIRST + 1;

While you're here, I think we should replace:

 /* ALL_COP_REG_NUM assumes that COP0,2,and 3 are numbered consecutively.  */
 #define ALL_COP_REG_NUM (COP3_REG_LAST - COP0_REG_FIRST + 1)

with:

 /* These definitions assume that COP0, 2 and 3 are numbered consecutively.  */
 #define ALL_COP_REG_FIRST COP0_REG_FIRST
 #define ALL_COP_REG_LAST COP3_REG_LAST
 #define ALL_COP_REG_NUM (ALL_COP_REG_LAST - ALL_COP_REG_FIRST + 1)

and the use ALL_COP_REG_{FIRST,LAST} in your new loop.  It feels wrong
to iterate with bounds from different ranges.

The MIPS parts are OK with that change.  Thanks for fixing this,
was going to try tackling it this weekend :-)

As far as the dwarf2out.c bits go, I think the original dbx_reg_number assert:

  gcc_assert (regno != INVALID_REGNUM);

should become:

  gcc_assert (regno != INVALID_REGNUM && regno != IGNORED_DWARF_REGNUM);

since it's the caller's job to handle this case.

Richard
H.J. Lu - Dec. 8, 2012, 4:05 p.m.
On Sat, Dec 8, 2012 at 2:14 AM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
>
> As far as the dwarf2out.c bits go, I think the original dbx_reg_number assert:
>
>   gcc_assert (regno != INVALID_REGNUM);
>
> should become:
>
>   gcc_assert (regno != INVALID_REGNUM && regno != IGNORED_DWARF_REGNUM);
>
> since it's the caller's job to handle this case.
>

I don't think if it will work since dbx_reg_number is called
to get debug register number and the register is ignored
if its return value is IGNORED_DWARF_REGNUM.

On the other hand, we should cache the return value of
dbx_reg_number(rtl), instead of calling it twice.
Richard Sandiford - Dec. 10, 2012, 10:53 a.m.
"H.J. Lu" <hjl.tools@gmail.com> writes:
> On Sat, Dec 8, 2012 at 2:14 AM, Richard Sandiford
> <rdsandiford@googlemail.com> wrote:
>>
>> As far as the dwarf2out.c bits go, I think the original dbx_reg_number assert:
>>
>>   gcc_assert (regno != INVALID_REGNUM);
>>
>> should become:
>>
>>   gcc_assert (regno != INVALID_REGNUM && regno != IGNORED_DWARF_REGNUM);
>>
>> since it's the caller's job to handle this case.
>>
>
> I don't think if it will work since dbx_reg_number is called
> to get debug register number and the register is ignored
> if its return value is IGNORED_DWARF_REGNUM.

Er, quite.  What was I thinking?

So please ignore that stupid suggestion.  The comments about the MIPS
parts still stand of course.

Richard

Patch

diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index b6a2290..bc99f29 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -16757,6 +16757,9 @@  mips_option_override (void)
   for (i = FP_REG_FIRST; i <= FP_REG_LAST; i++)
     mips_dbx_regno[i] = i + start;
 
+  for (i = COP0_REG_FIRST; i <= COP3_REG_LAST; i++)
+    mips_dbx_regno[i] = IGNORED_DWARF_REGNUM;
+
   /* Accumulator debug registers use big-endian ordering.  */
   mips_dbx_regno[HI_REGNUM] = MD_DBX_FIRST + 0;
   mips_dbx_regno[LO_REGNUM] = MD_DBX_FIRST + 1;
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index f0256ae..caedb5f 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -10473,6 +10473,9 @@  reg_loc_descriptor (rtx rtl, enum var_init_status initialized)
   if (REGNO (rtl) >= FIRST_PSEUDO_REGISTER)
     return 0;
 
+  if (dbx_reg_number(rtl) == IGNORED_DWARF_REGNUM)
+    return 0;
+
   /* We only use "frame base" when we're sure we're talking about the
      post-prologue local stack frame.  We do this by *not* running
      register elimination until this point, and recognizing the special
@@ -11931,6 +11934,8 @@  mem_loc_descriptor (rtx rtl, enum machine_mode mode,
 	    break;
 	  if (REGNO (rtl) > FIRST_PSEUDO_REGISTER)
 	    break;
+	  if (dbx_reg_number(rtl) == IGNORED_DWARF_REGNUM)
+	    break;
 	  type_die = base_type_for_mode (mode,
 					 GET_MODE_CLASS (mode) == MODE_INT);
 	  if (type_die == NULL)
@@ -12133,6 +12138,9 @@  mem_loc_descriptor (rtx rtl, enum machine_mode mode,
 	return NULL;
       if (REG_P (ENTRY_VALUE_EXP (rtl)))
 	{
+	  if (dbx_reg_number (ENTRY_VALUE_EXP (rtl)) == IGNORED_DWARF_REGNUM)
+	    return NULL;
+
 	  if (GET_MODE_CLASS (mode) != MODE_INT
 	      || GET_MODE_SIZE (mode) > DWARF2_ADDR_SIZE)
 	    op0 = mem_loc_descriptor (ENTRY_VALUE_EXP (rtl), mode,
diff --git a/gcc/rtl.h b/gcc/rtl.h
index a0fb4f7..5b9ceb8 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -2439,6 +2439,9 @@  extern rtx gen_rtx_MEM (enum machine_mode, rtx);
 /* REGNUM never really appearing in the INSN stream.  */
 #define INVALID_REGNUM			(~(unsigned int) 0)
 
+/* REGNUM for which no debug information can be generated.  */
+#define IGNORED_DWARF_REGNUM            (INVALID_REGNUM - 1)
+
 extern rtx output_constant_def (tree, int);
 extern rtx lookup_constant_def (tree);