diff mbox

[DWARF] Fix multiple register spanning location.

Message ID 51A37FCE.7010701@st.com
State New
Headers show

Commit Message

Christian Bruel May 27, 2013, 3:46 p.m. UTC
On 05/16/2013 12:27 AM, Cary Coutant wrote:
>>> How about using dbx_reg_number (XVECEXP (regs, 0, i)) instead? The
>>> bare use of DBX_REGISTER_NUMBER earlier in that function is protected
>>> by a gcc_assert, but this one isn't.
>>
For the respective targets maintainers that drop into the thread: Here
is a summary of the problem:

Since http://gcc.gnu.org/ml/gcc-patches/2012-03/msg00209.html, dwarf
double floating point registers are not correctly described for the SH.
But this patch was needed to fix an assertion in the dwarf2cfi.

Therefore, we have a discrepancy between the different targets, that can
result in assertions, (or possibly silent wrong unwind code I believe)

SH,MIPS,C6X   ; dwarf_register_span returns hard_reg numbers. regno is
never translated for DBX
ARM  NEON     ;                                  converts regno into DBX
numbers
POWERPC        ;                                 ? returns boths.

So a second set of patches
http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01230.html
http://gcc.gnu.org/ml/gcc-patches/2013-05/msg00312.html

fixed it with a common rule. All interfaces are changed to return
hard_reg numbers only. multiple_reg_location is in charge of calling 
DBX_REGISTER_NUMBER with an assertion check.

Well, in fact this was not doing some good for the powerpc, that now
asserts here. The problem is that rs6000_dwarf_register_span stores in
the PARALLEL rtx both the hard reg and the dbx reg, so we can't call
dbx_reg_number in it.
Using DBX_REGISTER_NUMBER instead of dbx_reg_number restores the
previous working status for all targets. This is
dwarf-span-assert-rs6000.patch for reference.

However I feel a little bit uncomfortable with this solution that
doesn't seem to fix the root cause. The dbx_register_number hooks is
called basically from two places : dwarf2cfi.c and dwarf2out.c. That
show different uses: either we want to refer to the hard regno when
dealing with the cfa description (whereas we want DWARF_FRAME_REGNUM,
not DBX_REGISTER_NUMBER). or we use the DBX_REGISTER_NUMBER for output
register locations.

Since this information cannot be detected contextually, I'd like to
extend the dwarf_register_span target hook  to return a dbx number or
not. This is dwarf-span-target-dbx.patch

build tested with the configurations that failed at one time or the other:
  - sh64-unknown-elf  (The original sh64-elf build failure assertion in
dwarf2cfi is fixed.)
  - arm-none-eabi -with-fpu=neon-vfpv4
  - powerpc-e500v2-linux-gnuspe
  - x86_64-unknown-linux-gnu sanity build OK

Is dwarf-span-target-dbx.patch OK for trunk ?. More comments ?

Many Thanks,

Christian

Comments

Kaz Kojima May 28, 2013, 11:19 p.m. UTC | #1
Christian Bruel <christian.bruel@st.com> wrote:
> However I feel a little bit uncomfortable with this solution that
> doesn't seem to fix the root cause. The dbx_register_number hooks is
> called basically from two places : dwarf2cfi.c and dwarf2out.c. That
> show different uses: either we want to refer to the hard regno when
> dealing with the cfa description (whereas we want DWARF_FRAME_REGNUM,
> not DBX_REGISTER_NUMBER). or we use the DBX_REGISTER_NUMBER for output
> register locations.
> 
> Since this information cannot be detected contextually, I'd like to
> extend the dwarf_register_span target hook  to return a dbx number or
> not. This is dwarf-span-target-dbx.patch
> 
> build tested with the configurations that failed at one time or the other:
>   - sh64-unknown-elf  (The original sh64-elf build failure assertion in
> dwarf2cfi is fixed.)
>   - arm-none-eabi -with-fpu=neon-vfpv4
>   - powerpc-e500v2-linux-gnuspe
>   - x86_64-unknown-linux-gnu sanity build OK
> 
> Is dwarf-span-target-dbx.patch OK for trunk ?. More comments ?

SH portion looks fine.  I've tested the patch with the top level
"make -k check" also on sh4-unknown-linux-gnu with no new failures.  

Regards,
	kaz
Christian Bruel June 11, 2013, 7:43 a.m. UTC | #2
Hello,

May I have a review from the DWARF, the ARM and RS6000 maintainers for
comments/approval ?

http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01613.html

Needed to fix the powerpc-spe bootstrap referenced in bugzilla #57389

Many Thanks

Christian


Christian Bruel <christian.bruel@st.com> wrote:
>> However I feel a little bit uncomfortable with this solution that
>> doesn't seem to fix the root cause. The dbx_register_number hooks is
>> called basically from two places : dwarf2cfi.c and dwarf2out.c. That
>> show different uses: either we want to refer to the hard regno when
>> dealing with the cfa description (whereas we want DWARF_FRAME_REGNUM,
>> not DBX_REGISTER_NUMBER). or we use the DBX_REGISTER_NUMBER for output
>> register locations.
>>
>> Since this information cannot be detected contextually, I'd like to
>> extend the dwarf_register_span target hook  to return a dbx number or
>> not. This is dwarf-span-target-dbx.patch
>>
>> build tested with the configurations that failed at one time or the other:
>>   - sh64-unknown-elf  (The original sh64-elf build failure assertion in
>> dwarf2cfi is fixed.)
>>   - arm-none-eabi -with-fpu=neon-vfpv4
>>   - powerpc-e500v2-linux-gnuspe
>>   - x86_64-unknown-linux-gnu sanity build OK
>>
>> Is dwarf-span-target-dbx.patch OK for trunk ?. More comments ?
diff mbox

Patch

2013-05-23  Christian Bruel  <christian.bruel@st.com>

	PR debug/57389
        * dwarf2out.c (multiple_reg_loc_descriptor): Use DBX_REGISTER_NUMBER
	instead	of dbx_reg_number.

Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	(revision 199354)
+++ gcc/dwarf2out.c	(working copy)
@@ -10660,8 +10660,11 @@  multiple_reg_loc_descriptor (rtx rtl, rtx regs,
     {
       dw_loc_descr_ref t;
 
-      t = one_reg_loc_descriptor (dbx_reg_number (XVECEXP (regs, 0, i)),
+      /* Cannot use dbx_reg_number here because regno could be out of the hard-reg
+	 range and not handled by DBX_REGISTER_NUMBER. See rs6000.h.  */
+      t = one_reg_loc_descriptor (DBX_REGISTER_NUMBER (REGNO (XVECEXP (regs, 0, i))),
 				  VAR_INIT_STATUS_INITIALIZED);
+
       add_loc_descr (&loc_result, t);
       add_loc_descr_op_piece (&loc_result, size);
     }