[RFC: Patch, PR 60102] [4.9/4.10 Regression] powerpc fp-bit ices at dwf_regno

[r***@freescale.com]

Hello All,

This is related to the following bug:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60102

I have tried to fix the e500v2 build on GCC v4.9.0 with the attached patch.
Can you please review and comment on the changes especially DWARF_FRAME_REGNUM, DWARF_REG_TO_UNWIND_COLUMN definitions?

Tested this patch on trunk [r212120] with ppc64 and didn't find any new regressions.
Back ported this patch on GCC v4.8.2 e500v2 and tested with no new regressions

Note:
With GCC v4.9.0, to build the e500v2 bareboard version the attached patch is enough.
With GCC v4.9.0. to build the e500v2 linux version along with the attached patch please add pr60735 patch too.

Regards,
Rohit

[r***@freescale.com]

Ping!

> -----Original Message-----
> From: Dharmakan Rohit-B30502
> Sent: Tuesday, July 08, 2014 8:13 AM
> To: gcc-***@gcc.gnu.org
> Cc: Wienskoski Edmar-RA8797; ***@gmail.com; Alan Modra; Jakub Jelinek
> Subject: [RFC: Patch, PR 60102] [4.9/4.10 Regression] powerpc fp-bit ices at
> dwf_regno
>
> Hello All,
>
> This is related to the following bug:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60102
>
> I have tried to fix the e500v2 build on GCC v4.9.0 with the attached patch.
> Can you please review and comment on the changes especially
> DWARF_FRAME_REGNUM, DWARF_REG_TO_UNWIND_COLUMN definitions?
>
> Tested this patch on trunk [r212120] with ppc64 and didn't find any new
> regressions.
> Back ported this patch on GCC v4.8.2 e500v2 and tested with no new
> regressions
>
> Note:
> With GCC v4.9.0, to build the e500v2 bareboard version the attached patch is
> enough.
> With GCC v4.9.0. to build the e500v2 linux version along with the attached
> patch please add pr60735 patch too.
>
> Regards,
> Rohit
>

[Ulrich Weigand]

Rohit wrote:

> This is related to the following bug:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D60102
>
> I have tried to fix the e500v2 build on GCC v4.9.0 with the attached patch.
> Can you please review and comment on the changes especially DWARF_FRAME_REG=
> NUM, DWARF_REG_TO_UNWIND_COLUMN definitions?

David asked me to comment on the use of DWARF register numbers in this patch.

There's a number of register number "address spaces" in play here:

(A) GCC hard register numbers
(B) DWARF register numbers used in .debug_info etc.
(C) DWARF CFI register numbers (GCC internal)
(D) DWARF CFI register numbers as used in .debug_frame
(E) DWARF CFI register numbers as used in .eh_frame
(F) DWARF CFI unwind column numbers

These are a number of macros to convert between them:

DBX_REGISTER_NUMBER: (A) -> (B)
DWARF_FRAME_REGNUM: (A) -> (C)
DWARF2_FRAME_REG_OUT: (C) -> (D) / (E)
DWARF_REG_TO_UNWIND_COLUMN: (E) -> (F)


Note that some of these seem to be used incorrectly in current rs6000.c:

for (i = FIRST_ALTIVEC_REGNO; i < LAST_ALTIVEC_REGNO+1; i++)
{
int column = DWARF_REG_TO_UNWIND_COLUMN (i);
HOST_WIDE_INT offset
= DWARF_FRAME_REGNUM (column) * GET_MODE_SIZE (mode);

This should rather be

int column = DWARF_REG_TO_UNWIND_COLUMN (DWARF_FRAME_REGNUM (i));
HOST_WIDE_INT offset = column * GET_MODE_SIZE (mode);

which doesn't show up as problem currently since DWARF_FRAME_REGNUM
is defined as the identity mapping, but will show up once you have to
actually define a nontrivial mapping in DWARF_FRAME_REGNUM.

[ To be fully correct, I guess it actually should be

int column = DWARF_REG_TO_UNWIND_COLUMN
(DWARF2_FRAME_REG_OUT (DWARF_FRAME_REGNUM (i), true));

but DWARF2_FRAME_REG_OUT (..., true) is the identity map as well ... ]


Now, if I understand the SPE situation correctly, you had previously:

- no GCC hard register numbers
(however, rs6000_dwarf_register_span, which is supposed to return a
hard register number, returned numbers in the 1200..1231 range)
- used the 1200..1231 range for (B), (C), (D), and (E)
- used the 113..145 range for (F)

Now, you need to introduce new GCC hard register numbers (A). However, in
order to preserve compatibility with DWARF info in existing binaries, none
of (B), (D), (E) or (F) is allowed to change. [ (C) could change in theory,
but it's probably best not to change it either. ]

Your patch now defines the new GCC hard register numbers in the 117..149 range,
which seems reasonable. However, you ought to the leave the other mappings
unchanged. For (B) this looks OK due to the rs6000_dbx_register_number change.

However (C), (D), and (E) *do* change with your patch:

> -#define DWARF_FRAME_REGNUM(REGNO) (REGNO)
> +#define DWARF_FRAME_REGNUM(REGNO) \
> + ((REGNO) >= 1200 ? ((REGNO) - 1200 + (DWARF_FRAME_REGISTERS - 32)) : (REGNO))

This isn't OK; the input to DWARF_FRAME_REGNUM is a GCC hard register number,
which will never be in the 1200... range.

On the other hand, you can now get hard register numbers in the 117..149 range,
which you need to map *back* to the 1200..1231 range, or else CFI register
numbers will be wrong. So you should have something like:

#define DWARF_FRAME_REGNUM(REGNO) \
(SPE_HIGH_REGNO_P(REGNO)? ((REGNO) - FIRST_SPE_HIGH_REGNO + 1200) : (REGNO))

On the other hand, the DWARF_REG_TO_UNWIND_COLUMN macro needs to map that
1200..1231 range back to the 113..145 range, so it should just stay as-is.

Note that (F) ends up being OK with your patch as-is, since the two bugs
in DWARF_FRAME_REGNUM and DWARF_REG_TO_UNWIND_COLUMN cancel each other out.


A couple of further comments on the patch:

> Index: libgcc/config/rs6000/linux-unwind.h
> ===================================================================
> --- libgcc/config/rs6000/linux-unwind.h (revision 212339)
> +++ libgcc/config/rs6000/linux-unwind.h (working copy)
> @@ -274,8 +274,8 @@ ppc_fallback_frame_state (struct _Unwind
> #ifdef __SPE__
> for (i = 14; i < 32; i++)
> {
> - fs->regs.reg[i + FIRST_PSEUDO_REGISTER - 1].how = REG_SAVED_OFFSET;
> - fs->regs.reg[i + FIRST_PSEUDO_REGISTER - 1].loc.offset
> + fs->regs.reg[i + FIRST_SPE_HIGH_REGNO - 4].how = REG_SAVED_OFFSET;
> + fs->regs.reg[i + FIRST_SPE_HIGH_REGNO - 4].loc.offset

This is a change to current behaviour, but that was probably intended
since the old behaviour seems broken (apparently wasn't updated after
the introduction of the three HTM registers).

> Index: gcc/config/rs6000/rs6000.c
> ===================================================================
> --- gcc/config/rs6000/rs6000.c (revision 212339)
> +++ gcc/config/rs6000/rs6000.c (working copy)
> @@ -30956,7 +30956,7 @@ rs6000_init_dwarf_reg_sizes_extra (tree
> rtx mem = gen_rtx_MEM (BLKmode, addr);
> rtx value = gen_int_mode (4, mode);
>
> - for (i = 1201; i < 1232; i++)
> + for (i = FIRST_SPE_HIGH_REGNO; i < LAST_SPE_HIGH_REGNO+1; i++)

Again this seems to change behaviour, but the old seems broken (didn't
initialize the first SPE high register).

> -/* Add 32 dwarf columns for synthetic SPE registers. */
> -#define DWARF_FRAME_REGISTERS ((FIRST_PSEUDO_REGISTER - 4) + 32)
> +/* SPE high registers added as hard regs.
> + The 3 HTM registers aren't included in DWARF_FRAME_REGISTERS */
> +#define DWARF_FRAME_REGISTERS (FIRST_PSEUDO_REGISTER - 4)

This is OK, but the comment is confusing: the -4 is because *four*
registers aren't included in DWARF_FRAME_REGISTER, namely the 3 HTM
registers *and the sfp register*.

> /* The SPE has an additional 32 synthetic registers, with DWARF debug
> info numbering for these registers starting at 1200. While eh_frame
> @@ -951,13 +952,14 @@ enum data_align { align_abi, align_opt,
> We must map them here to avoid huge unwinder tables mostly consisting
> of unused space. */
> #define DWARF_REG_TO_UNWIND_COLUMN(r) \
> - ((r) > 1200 ? ((r) - 1200 + (DWARF_FRAME_REGISTERS - 32)) : (r))
> + ((r) >= FIRST_SPE_HIGH_REGNO ? ((r) - FIRST_SPE_HIGH_REGNO + (DWARF_FRAME_REGISTERS - 32)) : (r))

As discussed above, this shouldn't change.

> /* Use gcc hard register numbering for eh_frame. */
> -#define DWARF_FRAME_REGNUM(REGNO) (REGNO)
> +#define DWARF_FRAME_REGNUM(REGNO) \
> + ((REGNO) >= 1200 ? ((REGNO) - 1200 + (DWARF_FRAME_REGISTERS - 32)) : (REGNO))

As discussed above, this is wrong.

> + { 0x00000000, 0x00000000, 0x00000000, 0xfff00000, 0x000fffff }, /* SPE_HIGH_REGS */ \
> + { 0xffffffff, 0xffffffff, 0xfffffffe, 0xfff7ffff, 0x000fffff } /* ALL_REGS */ \

This looks wrong to me; the SPE high regs have hard register numbers
in the 117..149 range. 117 is not a multiple of 4, so there cannot
be just "f" hex characters in the map for SPE_HIGH_REGS.


> + &rs6000_reg_names[117][0], /* SPE rh0 */ \
> + &rs6000_reg_names[118][0], /* SPE rh1 */ \
> + &rs6000_reg_names[119][0], /* SPE rh2 */ \

You need to actually initialize those rs6000_reg_names field in rs6000.c
if you refer to them here.


Bye,
Ulrich


--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
***@de.ibm.com

[r***@freescale.com]

Ulrich,

Thanks for your comments, I have updated the patch accordingly.

> > /* The SPE has an additional 32 synthetic registers, with DWARF debug
> > info numbering for these registers starting at 1200. While
> > eh_frame @@ -951,13 +952,14 @@ enum data_align { align_abi, align_opt,
> > We must map them here to avoid huge unwinder tables mostly consisting
> > of unused space. */
> > #define DWARF_REG_TO_UNWIND_COLUMN(r) \
> > - ((r) > 1200 ? ((r) - 1200 + (DWARF_FRAME_REGISTERS - 32)) : (r))
> > + ((r) >= FIRST_SPE_HIGH_REGNO ? ((r) - FIRST_SPE_HIGH_REGNO +
> > + (DWARF_FRAME_REGISTERS - 32)) : (r))
>
> As discussed above, this shouldn't change.

Updated to handle first SPE high register too.

#define DWARF_REG_TO_UNWIND_COLUMN(r) \
- ((r) > 1200 ? ((r) - 1200 + (DWARF_FRAME_REGISTERS - 32)) : (r))
+ ((r) >= 1200 ? ((r) - 1200 + (DWARF_FRAME_REGISTERS - 32)) : (r)

Tested this patch on trunk [213030] & GCC v4.9.1 with ppc64 and didn't find any new regressions.
Back ported this patch on GCC v4.8.3 e500v2 and tested with no new regressions

PR target/60102

[libgcc]
2014-07-31 Rohit <***@freescale.com>
* config/rs6000/linux-unwind.h (ppc_fallback_frame_state): Update
based on change in SPE high register numbers and 3 HTM registers.

[gcc]
2014-07-31 Rohit <***@freescale.com>
* config/rs6000/rs6000.c
(rs6000_reg_names) : Add SPE high register names.
(alt_reg_names) : Likewise
(rs6000_dwarf_register_span) : For SPE high registers, replace
dwarf register numbers with GCC hard register numbers.
(rs6000_init_dwarf_reg_sizes_extra) : Likewise.
(rs6000_dbx_register_number): For SPE high registers, return dwarf
register number for the corresponding GCC hard register number.

* config/rs6000/rs6000.h
(FIRST_PSEUDO_REGISTER) : Update based on 32 newly added GCC hard
register numbers for SPE high registers.
(DWARF_FRAME_REGISTERS) : Likewise.
(DWARF_REG_TO_UNWIND_COLUMN) : Likewise.
(DWARF_FRAME_REGNUM) : Likewise.
(FIXED_REGISTERS) : Likewise.
(CALL_USED_REGISTERS) : Likewise.
(CALL_REALLY_USED_REGISTERS) : Likewise.
(REG_ALLOC_ORDER) : Likewise.
(enum reg_class) : Likewise.
(REG_CLASS_NAMES) : Likewise.
(REG_CLASS_CONTENTS) : Likewise.
(SPE_HIGH_REGNO_P) : New macro to identify SPE high registers.

* gcc.target/powerpc/pr60102.c: New testcase.

Please let me know if you have any further comments on the updated patch.

Regards,
Rohit

[Ulrich Weigand]

Rohit,

> #define DWARF_REG_TO_UNWIND_COLUMN(r) \
>- ((r) > 1200 ? ((r) - 1200 + (DWARF_FRAME_REGISTERS - 32)) : (r))
>+ ((r) >= 1200 ? ((r) - 1200 + (DWARF_FRAME_REGISTERS - 32)) : (r))

OK, makes sense.

> /* Use gcc hard register numbering for eh_frame. */
>-#define DWARF_FRAME_REGNUM(REGNO) (REGNO)
>+#define DWARF_FRAME_REGNUM(REGNO) \
>+ ((REGNO) >= FIRST_SPE_HIGH_REGNO ? ((REGNO) - FIRST_SPE_HIGH_REGNO + 1200) : (REGNO))

Any reason for not using SPE_HIGH_REGNO_P here, just in case we
do get other hard registers at some point?

Otherwise this now looks good to me. (Of course, I cannot
approve the patch myself.)

Thanks,
Ulrich

--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
***@de.ibm.com

[r***@freescale.com]

Hello Ulrich,

Thanks.

> > /* Use gcc hard register numbering for eh_frame. */ -#define
> >DWARF_FRAME_REGNUM(REGNO) (REGNO)
> >+#define DWARF_FRAME_REGNUM(REGNO) \
> >+ ((REGNO) >= FIRST_SPE_HIGH_REGNO ? ((REGNO) -
> FIRST_SPE_HIGH_REGNO +
> >+1200) : (REGNO))
>
> Any reason for not using SPE_HIGH_REGNO_P here, just in case we do get
> other hard registers at some point?

Yes, we can use it. I just have to move the definition of "SPE_HIGH_REGNO_P" macro before "DWARF_FRAME_REGNUM" macro definition.
[Previously, I had defined and placed "SPE_HIGH_REGNO_P" macro along with similar macros "ALTIVEC_REGNO_P" etc.]

I had updated the patch as required (For this last change, I have checked/tested only the builds: ppc64 trunk, e500v2 v4.9.1 bareboard & linux build).

PR target/60102

[libgcc]
2014-07-31 Rohit <***@freescale.com>
* config/rs6000/linux-unwind.h (ppc_fallback_frame_state): Update
based on change in SPE high register numbers and 3 HTM registers.

[gcc]
2014-07-31 Rohit <***@freescale.com>
* config/rs6000/rs6000.c
(rs6000_reg_names) : Add SPE high register names.
(alt_reg_names) : Likewise.
(rs6000_dwarf_register_span) : For SPE high registers, replace
dwarf register numbers with GCC hard register numbers.
(rs6000_init_dwarf_reg_sizes_extra) : Likewise.
(rs6000_dbx_register_number): For SPE high registers, return dwarf
register number for the corresponding GCC hard register number.

* config/rs6000/rs6000.h
(FIRST_PSEUDO_REGISTER) : Update based on 32 newly added GCC hard
register numbers for SPE high registers.
(DWARF_FRAME_REGISTERS) : Likewise.
(DWARF_REG_TO_UNWIND_COLUMN) : Likewise.
(DWARF_FRAME_REGNUM) : Likewise.
(FIXED_REGISTERS) : Likewise.
(CALL_USED_REGISTERS) : Likewise.
(CALL_REALLY_USED_REGISTERS) : Likewise.
(REG_ALLOC_ORDER) : Likewise.
(enum reg_class) : Likewise.
(REG_CLASS_NAMES) : Likewise.
(REG_CLASS_CONTENTS) : Likewise.
(SPE_HIGH_REGNO_P) : New macro to identify SPE high registers.

* gcc.target/powerpc/pr60102.c: New testcase.

Regards,
Rohit

[Jakub Jelinek]

On Fri, Aug 01, 2014 at 06:03:56PM +0000, ***@freescale.com wrote:
> PR target/60102

--- libgcc/config/rs6000/linux-unwind.h (revision 213110)
+++ libgcc/config/rs6000/linux-unwind.h (working copy)
@@ -274,8 +274,8 @@ ppc_fallback_frame_state (struct _Unwind
#ifdef __SPE__
for (i = 14; i < 32; i++)
{
- fs->regs.reg[i + FIRST_PSEUDO_REGISTER - 1].how = REG_SAVED_OFFSET;
- fs->regs.reg[i + FIRST_PSEUDO_REGISTER - 1].loc.offset
+ fs->regs.reg[i + FIRST_SPE_HIGH_REGNO - 4].how = REG_SAVED_OFFSET;
+ fs->regs.reg[i + FIRST_SPE_HIGH_REGNO - 4].loc.offset
= (long) &regs->vregs - new_cfa + 4 * i;
}
#endif

is a different index, previously i + 116, newly i + 113, is that
intentional?

Jakub

[r***@freescale.com]

Jakub,

> On Fri, Aug 01, 2014 at 06:03:56PM +0000, ***@freescale.com wrote:
> > PR target/60102
>
> --- libgcc/config/rs6000/linux-unwind.h (revision 213110)
> +++ libgcc/config/rs6000/linux-unwind.h (working copy)
> @@ -274,8 +274,8 @@ ppc_fallback_frame_state (struct _Unwind
> #ifdef __SPE__
> for (i = 14; i < 32; i++)
> {
> - fs->regs.reg[i + FIRST_PSEUDO_REGISTER - 1].how = REG_SAVED_OFFSET;
> - fs->regs.reg[i + FIRST_PSEUDO_REGISTER - 1].loc.offset
> + fs->regs.reg[i + FIRST_SPE_HIGH_REGNO - 4].how = REG_SAVED_OFFSET;
> + fs->regs.reg[i + FIRST_SPE_HIGH_REGNO - 4].loc.offset
> = (long) &regs->vregs - new_cfa + 4 * i;
> }
> #endif
>
> is a different index, previously i + 116, newly i + 113, is that intentional?
>

Yes, it is intentional.
This part of code wasn't updated after the introduction of three HTM registers.

Regards,
Rohit

[David Edelsohn]

On Fri, Aug 1, 2014 at 2:03 PM, ***@freescale.com
<***@freescale.com> wrote:
> Hello Ulrich,
>
> Thanks.
>
>> > /* Use gcc hard register numbering for eh_frame. */ -#define
>> >DWARF_FRAME_REGNUM(REGNO) (REGNO)
>> >+#define DWARF_FRAME_REGNUM(REGNO) \
>> >+ ((REGNO) >= FIRST_SPE_HIGH_REGNO ? ((REGNO) -
>> FIRST_SPE_HIGH_REGNO +
>> >+1200) : (REGNO))
>>
>> Any reason for not using SPE_HIGH_REGNO_P here, just in case we do get
>> other hard registers at some point?
>
> Yes, we can use it. I just have to move the definition of "SPE_HIGH_REGNO_P" macro before "DWARF_FRAME_REGNUM" macro definition.
> [Previously, I had defined and placed "SPE_HIGH_REGNO_P" macro along with similar macros "ALTIVEC_REGNO_P" etc.]
>
> I had updated the patch as required (For this last change, I have checked/tested only the builds: ppc64 trunk, e500v2 v4.9.1 bareboard & linux build).
>
> PR target/60102
>
> [libgcc]
> 2014-07-31 Rohit <***@freescale.com>
> * config/rs6000/linux-unwind.h (ppc_fallback_frame_state): Update
> based on change in SPE high register numbers and 3 HTM registers.
>
> [gcc]
> 2014-07-31 Rohit <***@freescale.com>
> * config/rs6000/rs6000.c
> (rs6000_reg_names) : Add SPE high register names.
> (alt_reg_names) : Likewise.
> (rs6000_dwarf_register_span) : For SPE high registers, replace
> dwarf register numbers with GCC hard register numbers.
> (rs6000_init_dwarf_reg_sizes_extra) : Likewise.
> (rs6000_dbx_register_number): For SPE high registers, return dwarf
> register number for the corresponding GCC hard register number.
>
> * config/rs6000/rs6000.h
> (FIRST_PSEUDO_REGISTER) : Update based on 32 newly added GCC hard
> register numbers for SPE high registers.
> (DWARF_FRAME_REGISTERS) : Likewise.
> (DWARF_REG_TO_UNWIND_COLUMN) : Likewise.
> (DWARF_FRAME_REGNUM) : Likewise.
> (FIXED_REGISTERS) : Likewise.
> (CALL_USED_REGISTERS) : Likewise.
> (CALL_REALLY_USED_REGISTERS) : Likewise.
> (REG_ALLOC_ORDER) : Likewise.
> (enum reg_class) : Likewise.
> (REG_CLASS_NAMES) : Likewise.
> (REG_CLASS_CONTENTS) : Likewise.
> (SPE_HIGH_REGNO_P) : New macro to identify SPE high registers.
>
> * gcc.target/powerpc/pr60102.c: New testcase.

The patch is okay with me if Uli is satisfied.

Thanks, David

[Ulrich Weigand]

David Edelsohn wrote:
> On Fri, Aug 1, 2014 at 2:03 PM, ***@freescale.com
> <***@freescale.com> wrote:
> > [libgcc]
> > 2014-07-31 Rohit <***@freescale.com>
> > * config/rs6000/linux-unwind.h (ppc_fallback_frame_state): Update
> > based on change in SPE high register numbers and 3 HTM registers.
> >
> > [gcc]
> > 2014-07-31 Rohit <***@freescale.com>
> > * config/rs6000/rs6000.c
> > (rs6000_reg_names) : Add SPE high register names.
> > (alt_reg_names) : Likewise.
> > (rs6000_dwarf_register_span) : For SPE high registers, replace
> > dwarf register numbers with GCC hard register numbers.
> > (rs6000_init_dwarf_reg_sizes_extra) : Likewise.
> > (rs6000_dbx_register_number): For SPE high registers, return dwarf
> > register number for the corresponding GCC hard register number.
> >
> > * config/rs6000/rs6000.h
> > (FIRST_PSEUDO_REGISTER) : Update based on 32 newly added GCC hard
> > register numbers for SPE high registers.
> > (DWARF_FRAME_REGISTERS) : Likewise.
> > (DWARF_REG_TO_UNWIND_COLUMN) : Likewise.
> > (DWARF_FRAME_REGNUM) : Likewise.
> > (FIXED_REGISTERS) : Likewise.
> > (CALL_USED_REGISTERS) : Likewise.
> > (CALL_REALLY_USED_REGISTERS) : Likewise.
> > (REG_ALLOC_ORDER) : Likewise.
> > (enum reg_class) : Likewise.
> > (REG_CLASS_NAMES) : Likewise.
> > (REG_CLASS_CONTENTS) : Likewise.
> > (SPE_HIGH_REGNO_P) : New macro to identify SPE high registers.
> >
> > * gcc.target/powerpc/pr60102.c: New testcase.
>
> The patch is okay with me if Uli is satisfied.

Yes, this is fine with me.

Bye,
Ulrich

--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
***@de.ibm.com

[Edmar]

Committed on trunk, revision 213596
Committed on 4.9 branch, revision 213597

I made an omission on the first commit. I did not add
the test case and corresponding ChangeLog entry.
Committed as obvious on trunk, revision 213598

Thanks
Edmar


On 08/04/2014 05:25 AM, Ulrich Weigand wrote:
> David Edelsohn wrote:
>> On Fri, Aug 1, 2014 at 2:03 PM, ***@freescale.com
>> <***@freescale.com> wrote:
>>> [libgcc]
>>> 2014-07-31 Rohit<***@freescale.com>
>>> * config/rs6000/linux-unwind.h (ppc_fallback_frame_state): Update
>>> based on change in SPE high register numbers and 3 HTM registers.
>>>
>>> [gcc]
>>> 2014-07-31 Rohit<***@freescale.com>
>>> * config/rs6000/rs6000.c
>>> (rs6000_reg_names) : Add SPE high register names.
>>> (alt_reg_names) : Likewise.
>>> (rs6000_dwarf_register_span) : For SPE high registers, replace
>>> dwarf register numbers with GCC hard register numbers.
>>> (rs6000_init_dwarf_reg_sizes_extra) : Likewise.
>>> (rs6000_dbx_register_number): For SPE high registers, return dwarf
>>> register number for the corresponding GCC hard register number.
>>>
>>> * config/rs6000/rs6000.h
>>> (FIRST_PSEUDO_REGISTER) : Update based on 32 newly added GCC hard
>>> register numbers for SPE high registers.
>>> (DWARF_FRAME_REGISTERS) : Likewise.
>>> (DWARF_REG_TO_UNWIND_COLUMN) : Likewise.
>>> (DWARF_FRAME_REGNUM) : Likewise.
>>> (FIXED_REGISTERS) : Likewise.
>>> (CALL_USED_REGISTERS) : Likewise.
>>> (CALL_REALLY_USED_REGISTERS) : Likewise.
>>> (REG_ALLOC_ORDER) : Likewise.
>>> (enum reg_class) : Likewise.
>>> (REG_CLASS_NAMES) : Likewise.
>>> (REG_CLASS_CONTENTS) : Likewise.
>>> (SPE_HIGH_REGNO_P) : New macro to identify SPE high registers.
>>>
>>> * gcc.target/powerpc/pr60102.c: New testcase.
>> The patch is okay with me if Uli is satisfied.
> Yes, this is fine with me.
>
> Bye,
> Ulrich
>

[Jakub Jelinek]

On Mon, Aug 04, 2014 at 11:51:34AM -0500, Edmar wrote:
> Committed on trunk, revision 213596
> Committed on 4.9 branch, revision 213597

Note the ChangeLog entry was grossly misformated.
I've fixed it up in gcc/ChangeLog on the trunk, but not on the branch
nor in libgcc. There should be no space before :, all lines
in ChangeLog entry should be just tab indented rather than tab + 2 spaces,
and filenames, unless they are too long, shouldn't be alone on the lines.
And testsuite entries shouldn't go into gcc/ChangeLog.

> On 08/04/2014 05:25 AM, Ulrich Weigand wrote:
> >David Edelsohn wrote:
> >>On Fri, Aug 1, 2014 at 2:03 PM, ***@freescale.com
> >><***@freescale.com> wrote:
> >>>[libgcc]
> >>>2014-07-31 Rohit<***@freescale.com>
> >>> * config/rs6000/linux-unwind.h (ppc_fallback_frame_state): Update
> >>> based on change in SPE high register numbers and 3 HTM registers.
> >>>
> >>>[gcc]
> >>>2014-07-31 Rohit<***@freescale.com>
> >>> * config/rs6000/rs6000.c
> >>> (rs6000_reg_names) : Add SPE high register names.
> >>> (alt_reg_names) : Likewise.
> >>> (rs6000_dwarf_register_span) : For SPE high registers, replace
> >>> dwarf register numbers with GCC hard register numbers.
> >>> (rs6000_init_dwarf_reg_sizes_extra) : Likewise.
> >>> (rs6000_dbx_register_number): For SPE high registers, return dwarf
> >>> register number for the corresponding GCC hard register number.
> >>>
> >>> * config/rs6000/rs6000.h
> >>> (FIRST_PSEUDO_REGISTER) : Update based on 32 newly added GCC hard
> >>> register numbers for SPE high registers.
> >>> (DWARF_FRAME_REGISTERS) : Likewise.
> >>> (DWARF_REG_TO_UNWIND_COLUMN) : Likewise.
> >>> (DWARF_FRAME_REGNUM) : Likewise.
> >>> (FIXED_REGISTERS) : Likewise.
> >>> (CALL_USED_REGISTERS) : Likewise.
> >>> (CALL_REALLY_USED_REGISTERS) : Likewise.
> >>> (REG_ALLOC_ORDER) : Likewise.
> >>> (enum reg_class) : Likewise.
> >>> (REG_CLASS_NAMES) : Likewise.
> >>> (REG_CLASS_CONTENTS) : Likewise.
> >>> (SPE_HIGH_REGNO_P) : New macro to identify SPE high registers.
> >>>
> >>> * gcc.target/powerpc/pr60102.c: New testcase.
> >>The patch is okay with me if Uli is satisfied.
> >Yes, this is fine with me.

Jakub

[r***@freescale.com]

Jakub,

> On Mon, Aug 04, 2014 at 11:51:34AM -0500, Edmar wrote:
> > Committed on trunk, revision 213596
> > Committed on 4.9 branch, revision 213597
>
> Note the ChangeLog entry was grossly misformated.
> I've fixed it up in gcc/ChangeLog on the trunk, but not on the branch
> nor in libgcc. There should be no space before :, all lines
> in ChangeLog entry should be just tab indented rather than tab + 2 spaces,
> and filenames, unless they are too long, shouldn't be alone on the lines.
> And testsuite entries shouldn't go into gcc/ChangeLog.

Sorry about that. I have updated the ChangeLog entries accordingly.

Edmar, can you please commit these patches?

Trunk: pr60102-ChangeLog-trunk.patch
GCC v4.9 branch: pr60102-ChangeLog-v49.patch

Regards,
Rohit

[Edmar]

Jackub,

Thanks for point this up.
I apologize for the sloppiness.

I fixed and committed the ChangeLogs on the branch, revision 213639
Also fixed the libgcc ChangeLog on trunk. Revision 213640

Edmar


On 08/05/2014 03:11 AM, Jakub Jelinek wrote:
> On Mon, Aug 04, 2014 at 11:51:34AM -0500, Edmar wrote:
>> Committed on trunk, revision 213596
>> Committed on 4.9 branch, revision 213597
> Note the ChangeLog entry was grossly misformated.
> I've fixed it up in gcc/ChangeLog on the trunk, but not on the branch
> nor in libgcc. There should be no space before :, all lines
> in ChangeLog entry should be just tab indented rather than tab + 2 spaces,
> and filenames, unless they are too long, shouldn't be alone on the lines.
> And testsuite entries shouldn't go into gcc/ChangeLog.
>
>> On 08/04/2014 05:25 AM, Ulrich Weigand wrote:
>>> David Edelsohn wrote:
>>>> On Fri, Aug 1, 2014 at 2:03 PM, ***@freescale.com
>>>> <***@freescale.com> wrote:
>>>>> [libgcc]
>>>>> 2014-07-31 Rohit<***@freescale.com>
>>>>> * config/rs6000/linux-unwind.h (ppc_fallback_frame_state): Update
>>>>> based on change in SPE high register numbers and 3 HTM registers.
>>>>>
>>>>> [gcc]
>>>>> 2014-07-31 Rohit<***@freescale.com>
>>>>> * config/rs6000/rs6000.c
>>>>> (rs6000_reg_names) : Add SPE high register names.
>>>>> (alt_reg_names) : Likewise.
>>>>> (rs6000_dwarf_register_span) : For SPE high registers, replace
>>>>> dwarf register numbers with GCC hard register numbers.
>>>>> (rs6000_init_dwarf_reg_sizes_extra) : Likewise.
>>>>> (rs6000_dbx_register_number): For SPE high registers, return dwarf
>>>>> register number for the corresponding GCC hard register number.
>>>>>
>>>>> * config/rs6000/rs6000.h
>>>>> (FIRST_PSEUDO_REGISTER) : Update based on 32 newly added GCC hard
>>>>> register numbers for SPE high registers.
>>>>> (DWARF_FRAME_REGISTERS) : Likewise.
>>>>> (DWARF_REG_TO_UNWIND_COLUMN) : Likewise.
>>>>> (DWARF_FRAME_REGNUM) : Likewise.
>>>>> (FIXED_REGISTERS) : Likewise.
>>>>> (CALL_USED_REGISTERS) : Likewise.
>>>>> (CALL_REALLY_USED_REGISTERS) : Likewise.
>>>>> (REG_ALLOC_ORDER) : Likewise.
>>>>> (enum reg_class) : Likewise.
>>>>> (REG_CLASS_NAMES) : Likewise.
>>>>> (REG_CLASS_CONTENTS) : Likewise.
>>>>> (SPE_HIGH_REGNO_P) : New macro to identify SPE high registers.
>>>>>
>>>>> * gcc.target/powerpc/pr60102.c: New testcase.
>>>> The patch is okay with me if Uli is satisfied.
>>> Yes, this is fine with me.
> Jakub
> .
>

[Maciej W. Rozycki]

On Mon, 4 Aug 2014, Edmar wrote:

> Committed on trunk, revision 213596
> Committed on 4.9 branch, revision 213597

This change regressed GDB for e500v2 multilibs, presumably because it
does not understand the new DWARF register numbers and does not know how
to map them to hardware registers. Here's the full list of regressions
observed:

FAIL: gdb.base/store.exp: var double l; print old l, expecting -1
FAIL: gdb.base/store.exp: var double l; print old r, expecting -2
FAIL: gdb.base/store.exp: var double l; print incremented l, expecting 2
FAIL: gdb.base/store.exp: var doublest l; print old l, expecting -1
FAIL: gdb.base/store.exp: var doublest l; print old r, expecting -2
FAIL: gdb.base/store.exp: var doublest l; print new l, expecting 4
FAIL: gdb.base/store.exp: var doublest l; print incremented l, expecting 2
FAIL: gdb.base/store.exp: upvar double l; print old l, expecting -1
FAIL: gdb.base/store.exp: upvar double l; print old r, expecting -2
UNRESOLVED: gdb.base/store.exp: upvar double l; set l to 4
UNRESOLVED: gdb.base/store.exp: upvar double l; print new l, expecting 4
FAIL: gdb.base/store.exp: upvar doublest l; print old l, expecting -1
FAIL: gdb.base/store.exp: upvar doublest l; print old r, expecting -2
UNRESOLVED: gdb.base/store.exp: upvar doublest l; set l to 4
UNRESOLVED: gdb.base/store.exp: upvar doublest l; print new l, expecting 4

These tests all used to score a PASS status. Do you plan to address this
problem anyhow?

Maciej

[Ulrich Weigand]

Maciej W. Rozycki wrote:
> On Mon, 4 Aug 2014, Edmar wrote:
>
> > Committed on trunk, revision 213596
> > Committed on 4.9 branch, revision 213597
>
> This change regressed GDB for e500v2 multilibs, presumably because it
> does not understand the new DWARF register numbers and does not know how
> to map them to hardware registers.

As I understand it, the change was supposed to only affect GCC internals,
all externally generated debug info was supposed to remain unchanged.

If there are changes in debug info, something must have gone wrong.

Bye,
Ulrich

--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
***@de.ibm.com

[r***@freescale.com]

> From: Ulrich Weigand [mailto:***@de.ibm.com]
> Maciej W. Rozycki wrote:
> > On Mon, 4 Aug 2014, Edmar wrote:
> >
> > > Committed on trunk, revision 213596
> > > Committed on 4.9 branch, revision 213597
> >
> > This change regressed GDB for e500v2 multilibs, presumably because it
> > does not understand the new DWARF register numbers and does not know
> > how to map them to hardware registers.
>
> As I understand it, the change was supposed to only affect GCC internals, all
> externally generated debug info was supposed to remain unchanged.
>
> If there are changes in debug info, something must have gone wrong.

Let me check if I can track this down.

Regards,
Rohit

[Maciej W. Rozycki]

On Mon, 29 Sep 2014, ***@freescale.com wrote:

> > As I understand it, the change was supposed to only affect GCC internals, all
> > externally generated debug info was supposed to remain unchanged.
> >
> > If there are changes in debug info, something must have gone wrong.
>
> Let me check if I can track this down.

Thanks. In case that helps the multilib flags I used for this testing
were:

-mcpu=8548 -mfloat-gprs=double -mspe=yes -mabi=spe

Maciej

[r***@freescale.com]

> From: Dharmakan Rohit-B30502
> Sent: Monday, September 29, 2014 3:54 PM
>
> > From: Ulrich Weigand [mailto:***@de.ibm.com] Maciej W. Rozycki
> > wrote:
> > > On Mon, 4 Aug 2014, Edmar wrote:
> > >
> > > > Committed on trunk, revision 213596 Committed on 4.9 branch,
> > > > revision 213597
> > >
> > > This change regressed GDB for e500v2 multilibs, presumably because
> > > it does not understand the new DWARF register numbers and does not
> > > know how to map them to hardware registers.
> >
> > As I understand it, the change was supposed to only affect GCC
> > internals, all externally generated debug info was supposed to remain
> unchanged.
> >
> > If there are changes in debug info, something must have gone wrong.
>
> Let me check if I can track this down.

Maciej/Ulrich,

I was able to narrow down the issue.

Debug info generated with the current patch:
<2><334>: Abbrev Number: 10 (DW_TAG_formal_parameter)
<335> DW_AT_name : u
<337> DW_AT_decl_file : 1
<338> DW_AT_decl_line : 51
<339> DW_AT_type : <0x357>
<33d> DW_AT_location : 7 byte block: 90 7d 93 4 58 93 4 (DW_OP_regx: 125 (r125); DW_OP_piece: 4; DW_OP_reg8 (r8); DW_OP_piece: 4)

Expected debug info:
<2><334>: Abbrev Number: 10 (DW_TAG_formal_parameter)
<335> DW_AT_name : u
<337> DW_AT_decl_file : 1
<338> DW_AT_decl_line : 51
<339> DW_AT_type : <0x359>
<33d> DW_AT_location : 8 byte block: 90 b8 9 93 4 58 93 4 (DW_OP_regx: 1208 (r1208); DW_OP_piece: 4; DW_OP_reg8 (r8); DW_OP_piece: 4)

While emitting the location descriptors of multiple registers (SPE high/low part) individually, the GCC hard register number is converted in to DWARF register number using "dbx_reg_number" [Statement "A", "B" & "C" below].

File1: gcc-4.9/gcc/config/rs6000/rs6000.h
=================================

...
/* Use standard DWARF numbering for DWARF debugging information. */
#define DBX_REGISTER_NUMBER(REGNO) rs6000_dbx_register_number (REGNO) ---------------------------------------------------------(A)
...


File2: gcc-4.9/gcc/dwarf2out.c
=========================

/* Given an RTL of a register, return a location descriptor that
designates a value that spans more than one register. */

static dw_loc_descr_ref
multiple_reg_loc_descriptor (rtx rtl, rtx regs,
enum var_init_status initialized)
{

...
/* Now onto stupid register sets in non contiguous locations. */

....
for (i = 0; i < XVECLEN (regs, 0); ++i)
{
dw_loc_descr_ref t;
t = one_reg_loc_descriptor (dbx_reg_number (XVECEXP (regs, 0, i)), VAR_INIT_STATUS_INITIALIZED); -------------------------------(B)
...

}
.....

}
static unsigned int
dbx_reg_number (const_rtx rtl)
{
....
....
regno = DBX_REGISTER_NUMBER (regno); -------------------------------------------------------------------------------------------------------------(C)
gcc_assert (regno != INVALID_REGNUM);
return regno;
}

File3:gcc-4.9/gcc/config/rs6000/sysv4.h
===============================
....
/* This target uses the sysv4.opt file. */
#define TARGET_USES_SYSV4_OPT 1

#undef DBX_REGISTER_NUMBER ----------------------------------------------------------------------------------------------------------(D)
....

But statement "C" macro "DBX_REGISTER_NUMBER" gets undefined by statement "D" hence the GCC hard register number gets emitted in the debug info instead of DWARF register number.
Previously, without this patch for SPE high registers the GCC hard register number was same as the DWARF register number (1200 onwards), hence we didn't see this issue.

Removing statement "D" from "sysv4.h" file so as to generate expected DWARF register number does work, but will there be any side effects?

Regards,
Rohit

[Ulrich Weigand]

rohitarulraj wrote:

> I was able to narrow down the issue.
[snip]
> While emitting the location descriptors of multiple registers (SPE high/low
> part) individually, the GCC hard register number is converted in to DWARF
> register number using "dbx_reg_number" [Statement "A", "B" & "C" below].

> But statement "C" macro "DBX_REGISTER_NUMBER" gets undefined by statement
> "D" hence the GCC hard register number gets emitted in the debug info
> instead of DWARF register number. Previously, without this patch for SPE
> high registers the GCC hard register number was same as the DWARF register
> number (1200 onwards), hence we didn't see this issue.
>
> Removing statement "D" from "sysv4.h" file so as to generate expected
> DWARF register number does work, but will there be any side effects?

Ah, I had completely forgotten about this issue, sorry ...

The problem with DBX_REGISTER_NUMBER is actually described in detail here:
https://gcc.gnu.org/ml/gcc-patches/2012-11/msg02136.html

At the time, we decided to not remove the #undef DBX_REGISTER_NUMBER to
avoid compatibility issues, but use GCC internal numbers in .debug_frame
and .debug_info on Linux (option (3) in the above mail). However, this
was never actually implemented.

Looking at the current status, there are three groups of rs6000 targets:

- Some use the DBX_REGISTER_NUMBER definition from rs6000.h:
These are only AIX and Darwin.

- Some provide their own definition of DBX_REGISTER_NUMBER after the rs6000.h
one was undefined by sysv4.h:
These are FreeBSD, NetBSD, and Lynx.

- All other targets do not have DBX_REGISTER_NUMBER because it is undefined
by sysv4.h, and therefore using GCC internal register numbers:
These are Linux, rtems, vxworks, and all ELF/EABI targets.


The following patch tries to remove the unfortunate confusion about undefining
and redefining DBX_REGISTER_NUMBER, while keeping the behavior on all targets
unchanged with the following two exceptions:
- fix the SPE problem by always translating high register numbers
- implement option (3) above by not replacing CR2 with CR in .debug_frame
on targets that do not use the standard DWARF register numbering

The way this works is to have a common, simple implementation of
DBX_REGISTER_NUMBER and DWARF2_FRAME_REG_OUT for all targets that just
calls to the rs6000_dbx_register_number routine, passing an extra format
argument that decides whether the register number is to be used for
.debug_info, .debug_frame, or .eh_frame. In order to ensure
rs6000_dbx_register_number always gets a GCC internal number as input,
DWARF_FRAME_REGNUM has to be again defined as identity map.

All the logic to decide debug register numbers is now contained in that
single place. However, in order to maintain current behavior, we still
have to distinguish between platforms that want to use the standard
DWARF register numbering scheme, and those that use GCC internal numbers.
This is now simply done by having the former provide a new define
RS6000_USE_DWARF_NUMBERING in a target header file.

Tested on powerpc64le-linux and powerpc64-linux.

Rohit, could you verify whether this fixes the SPE problem?

David, does this approach look reasonable to you?


Bye,
Ulrich

ChangeLog:

* config/rs6000/rs6000.h (DBX_REGISTER_NUMBER): Pass format argument
to rs6000_dbx_register_number.
(DWARF_FRAME_REGNUM): Redefine as identity map.
(DWARF2_FRAME_REG_OUT): Call rs6000_dbx_register_number.
* config/rs6000/rs6000-protos.h (rs6000_dbx_register_number): Update.
* config/rs6000/rs6000.c (rs6000_dbx_register_number): Add format
argument to handle .debug_frame and .eh_frame directly. Always
translate SPE high register numbers. Add special treatment for CR,
but only in .debug_frame. Respect RS6000_USE_DWARF_NUMBERING.

* config/rs6000/sysv.h (DBX_REGISTER_NUMBER): Do not undefine.
* config/rs6000/freebsd.h (DBX_REGISTER_NUMBER): Remove.
(RS6000_USE_DWARF_NUMBERING): Define.
* config/rs6000/freebsd64.h (DBX_REGISTER_NUMBER): Remove.
(RS6000_USE_DWARF_NUMBERING): Define.
* config/rs6000/netbsd.h (DBX_REGISTER_NUMBER): Remove.
(RS6000_USE_DWARF_NUMBERING): Define.
* config/rs6000/lynx.h (DBX_REGISTER_NUMBER): Remove.
(RS6000_USE_DWARF_NUMBERING): Define.
* config/rs6000/aix.h (RS6000_USE_DWARF_NUMBERING): Define.
* config/rs6000/darwin.h (RS6000_USE_DWARF_NUMBERING): Define.


Index: gcc/config/rs6000/rs6000.h
===================================================================
--- gcc/config/rs6000/rs6000.h (revision 215999)
+++ gcc/config/rs6000/rs6000.h (working copy)
@@ -947,23 +947,16 @@ enum data_align { align_abi, align_opt,
((r) >= 1200 ? ((r) - 1200 + (DWARF_FRAME_REGISTERS - 32)) : (r))

/* Use standard DWARF numbering for DWARF debugging information. */
-#define DBX_REGISTER_NUMBER(REGNO) rs6000_dbx_register_number (REGNO)
+#define DBX_REGISTER_NUMBER(REGNO) rs6000_dbx_register_number ((REGNO), 0)

/* Use gcc hard register numbering for eh_frame. */
-#define DWARF_FRAME_REGNUM(REGNO) \
- (SPE_HIGH_REGNO_P (REGNO) ? ((REGNO) - FIRST_SPE_HIGH_REGNO + 1200) : (REGNO))
+#define DWARF_FRAME_REGNUM(REGNO) (REGNO)

/* Map register numbers held in the call frame info that gcc has
collected using DWARF_FRAME_REGNUM to those that should be output in
- .debug_frame and .eh_frame. We continue to use gcc hard reg numbers
- for .eh_frame, but use the numbers mandated by the various ABIs for
- .debug_frame. rs6000_emit_prologue has translated any combination of
- CR2, CR3, CR4 saves to a save of CR2. The actual code emitted saves
- the whole of CR, so we map CR2_REGNO to the DWARF reg for CR. */
-#define DWARF2_FRAME_REG_OUT(REGNO, FOR_EH) \
- ((FOR_EH) ? (REGNO) \
- : (REGNO) == CR2_REGNO ? 64 \
- : DBX_REGISTER_NUMBER (REGNO))
+ .debug_frame and .eh_frame. */
+#define DWARF2_FRAME_REG_OUT(REGNO, FOR_EH) \
+ rs6000_dbx_register_number ((REGNO), (FOR_EH)? 2 : 1)

/* 1 for registers that have pervasive standard uses
and are not available for the register allocator.
Index: gcc/config/rs6000/rs6000-protos.h
===================================================================
--- gcc/config/rs6000/rs6000-protos.h (revision 215999)
+++ gcc/config/rs6000/rs6000-protos.h (working copy)
@@ -188,7 +188,7 @@ extern int rs6000_trampoline_size (void)
extern alias_set_type get_TOC_alias_set (void);
extern void rs6000_emit_prologue (void);
extern void rs6000_emit_load_toc_table (int);
-extern unsigned int rs6000_dbx_register_number (unsigned int);
+extern unsigned int rs6000_dbx_register_number (unsigned int, unsigned int);
extern void rs6000_emit_epilogue (int);
extern void rs6000_emit_eh_reg_restore (rtx, rtx);
extern const char * output_isel (rtx *);
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c (revision 215999)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -31581,17 +31581,40 @@ rs6000_init_dwarf_reg_sizes_extra (tree
}
}

-/* Map internal gcc register numbers to DWARF2 register numbers. */
+/* Map internal gcc register numbers to debug format register numbers.
+ FORMAT specifies the type of debug register number to use:
+ 0 -- debug information, except for frame-related sections
+ 1 -- DWARF .debug_frame section
+ 2 -- DWARF .eh_frame section */

unsigned int
-rs6000_dbx_register_number (unsigned int regno)
+rs6000_dbx_register_number (unsigned int regno, unsigned int format)
{
- if (regno <= 63 || write_symbols != DWARF2_DEBUG)
+ /* We never use the GCC internal number for SPE high registers.
+ Those are mapped to the 1200..1231 range for all debug formats. */
+ if (SPE_HIGH_REGNO_P (regno))
+ return regno - FIRST_SPE_HIGH_REGNO + 1200;
+
+ /* Except for the above, we use the internal number for non-DWARF
+ debug information, and also for .eh_frame. */
+ if ((format == 0 && write_symbols != DWARF2_DEBUG) || format == 2)
+ return regno;
+
+ /* On some platforms, we use the standard DWARF register
+ numbering for .debug_info and .debug_frame. */
+#ifdef RS6000_USE_DWARF_NUMBERING
+ if (regno <= 63)
return regno;
if (regno == LR_REGNO)
return 108;
if (regno == CTR_REGNO)
return 109;
+ /* Special handling for CR for .debug_frame: rs6000_emit_prologue has
+ translated any combination of CR2, CR3, CR4 saves to a save of CR2.
+ The actual code emitted saves the whole of CR, so we map CR2_REGNO
+ to the DWARF reg for CR. */
+ if (format == 1 && regno == CR2_REGNO)
+ return 64;
if (CR_REGNO_P (regno))
return regno - CR0_REGNO + 86;
if (regno == CA_REGNO)
@@ -31606,8 +31629,7 @@ rs6000_dbx_register_number (unsigned int
return 99;
if (regno == SPEFSCR_REGNO)
return 612;
- if (SPE_HIGH_REGNO_P (regno))
- return regno - FIRST_SPE_HIGH_REGNO + 1200;
+#endif
return regno;
}

Index: gcc/config/rs6000/sysv4.h
===================================================================
--- gcc/config/rs6000/sysv4.h (revision 215999)
+++ gcc/config/rs6000/sysv4.h (working copy)
@@ -949,4 +949,3 @@ ncrtn.o%s"
/* This target uses the sysv4.opt file. */
#define TARGET_USES_SYSV4_OPT 1

-#undef DBX_REGISTER_NUMBER
Index: gcc/config/rs6000/freebsd.h
===================================================================
--- gcc/config/rs6000/freebsd.h (revision 215999)
+++ gcc/config/rs6000/freebsd.h (working copy)
@@ -73,6 +73,7 @@
#define RELOCATABLE_NEEDS_FIXUP \
(rs6000_isa_flags & rs6000_isa_flags_explicit & OPTION_MASK_RELOCATABLE)

-#define DBX_REGISTER_NUMBER(REGNO) rs6000_dbx_register_number (REGNO)
+/* Use standard DWARF numbering for DWARF debugging information. */
+#define RS6000_USE_DWARF_NUMBERING

#define POWERPC_FREEBSD
Index: gcc/config/rs6000/freebsd64.h
===================================================================
--- gcc/config/rs6000/freebsd64.h (revision 215999)
+++ gcc/config/rs6000/freebsd64.h (working copy)
@@ -362,7 +362,8 @@ extern int dot_symbols;
/* The default value isn't sufficient in 64-bit mode. */
#define STACK_CHECK_PROTECT (TARGET_64BIT ? 16 * 1024 : 12 * 1024)

-#define DBX_REGISTER_NUMBER(REGNO) rs6000_dbx_register_number (REGNO)
+/* Use standard DWARF numbering for DWARF debugging information. */
+#define RS6000_USE_DWARF_NUMBERING

/* PowerPC64 Linux word-aligns FP doubles when -malign-power is given. */
#undef ADJUST_FIELD_ALIGN
Index: gcc/config/rs6000/netbsd.h
===================================================================
--- gcc/config/rs6000/netbsd.h (revision 215999)
+++ gcc/config/rs6000/netbsd.h (working copy)
@@ -87,4 +87,6 @@
{ "netbsd_endfile_spec", NETBSD_ENDFILE_SPEC },


-#define DBX_REGISTER_NUMBER(REGNO) rs6000_dbx_register_number (REGNO)
+/* Use standard DWARF numbering for DWARF debugging information. */
+#define RS6000_USE_DWARF_NUMBERING
+
Index: gcc/config/rs6000/lynx.h
===================================================================
--- gcc/config/rs6000/lynx.h (revision 215999)
+++ gcc/config/rs6000/lynx.h (working copy)
@@ -99,7 +99,8 @@
#undef HAVE_AS_TLS
#define HAVE_AS_TLS 0

-#define DBX_REGISTER_NUMBER(REGNO) rs6000_dbx_register_number (REGNO)
+/* Use standard DWARF numbering for DWARF debugging information. */
+#define RS6000_USE_DWARF_NUMBERING

#ifdef CRT_BEGIN
/* This function is part of crtbegin*.o which is at the beginning of
Index: gcc/config/rs6000/aix.h
===================================================================
--- gcc/config/rs6000/aix.h (revision 215999)
+++ gcc/config/rs6000/aix.h (working copy)
@@ -223,3 +223,7 @@

/* Static stack checking is supported by means of probes. */
#define STACK_CHECK_STATIC_BUILTIN 1
+
+/* Use standard DWARF numbering for DWARF debugging information. */
+#define RS6000_USE_DWARF_NUMBERING
+
Index: gcc/config/rs6000/darwin.h
===================================================================
--- gcc/config/rs6000/darwin.h (revision 215999)
+++ gcc/config/rs6000/darwin.h (working copy)
@@ -424,3 +424,7 @@ do { \
/* So far, there is no rs6000_fold_builtin, if one is introduced, then
this will need to be modified similar to the x86 case. */
#define TARGET_FOLD_BUILTIN SUBTARGET_FOLD_BUILTIN
+
+/* Use standard DWARF numbering for DWARF debugging information. */
+#define RS6000_USE_DWARF_NUMBERING
+

--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
***@de.ibm.com

[David Edelsohn]

On Wed, Oct 8, 2014 at 2:09 PM, Ulrich Weigand <***@de.ibm.com> wrote:
> rohitarulraj wrote:
>
>> I was able to narrow down the issue.
> [snip]
>> While emitting the location descriptors of multiple registers (SPE high/low
>> part) individually, the GCC hard register number is converted in to DWARF
>> register number using "dbx_reg_number" [Statement "A", "B" & "C" below].
>
>> But statement "C" macro "DBX_REGISTER_NUMBER" gets undefined by statement
>> "D" hence the GCC hard register number gets emitted in the debug info
>> instead of DWARF register number. Previously, without this patch for SPE
>> high registers the GCC hard register number was same as the DWARF register
>> number (1200 onwards), hence we didn't see this issue.
>>
>> Removing statement "D" from "sysv4.h" file so as to generate expected
>> DWARF register number does work, but will there be any side effects?
>
> Ah, I had completely forgotten about this issue, sorry ...
>
> The problem with DBX_REGISTER_NUMBER is actually described in detail here:
> https://gcc.gnu.org/ml/gcc-patches/2012-11/msg02136.html
>
> At the time, we decided to not remove the #undef DBX_REGISTER_NUMBER to
> avoid compatibility issues, but use GCC internal numbers in .debug_frame
> and .debug_info on Linux (option (3) in the above mail). However, this
> was never actually implemented.
>
> Looking at the current status, there are three groups of rs6000 targets:
>
> - Some use the DBX_REGISTER_NUMBER definition from rs6000.h:
> These are only AIX and Darwin.
>
> - Some provide their own definition of DBX_REGISTER_NUMBER after the rs6000.h
> one was undefined by sysv4.h:
> These are FreeBSD, NetBSD, and Lynx.
>
> - All other targets do not have DBX_REGISTER_NUMBER because it is undefined
> by sysv4.h, and therefore using GCC internal register numbers:
> These are Linux, rtems, vxworks, and all ELF/EABI targets.
>
>
> The following patch tries to remove the unfortunate confusion about undefining
> and redefining DBX_REGISTER_NUMBER, while keeping the behavior on all targets
> unchanged with the following two exceptions:
> - fix the SPE problem by always translating high register numbers
> - implement option (3) above by not replacing CR2 with CR in .debug_frame
> on targets that do not use the standard DWARF register numbering
>
> The way this works is to have a common, simple implementation of
> DBX_REGISTER_NUMBER and DWARF2_FRAME_REG_OUT for all targets that just
> calls to the rs6000_dbx_register_number routine, passing an extra format
> argument that decides whether the register number is to be used for
> .debug_info, .debug_frame, or .eh_frame. In order to ensure
> rs6000_dbx_register_number always gets a GCC internal number as input,
> DWARF_FRAME_REGNUM has to be again defined as identity map.
>
> All the logic to decide debug register numbers is now contained in that
> single place. However, in order to maintain current behavior, we still
> have to distinguish between platforms that want to use the standard
> DWARF register numbering scheme, and those that use GCC internal numbers.
> This is now simply done by having the former provide a new define
> RS6000_USE_DWARF_NUMBERING in a target header file.
>
> Tested on powerpc64le-linux and powerpc64-linux.
>
> Rohit, could you verify whether this fixes the SPE problem?
>
> David, does this approach look reasonable to you?

This seems like the best solution. Thanks for untangling this.

- David

[Maciej W. Rozycki]

Ulrich,

> > While emitting the location descriptors of multiple registers (SPE high/low
> > part) individually, the GCC hard register number is converted in to DWARF
> > register number using "dbx_reg_number" [Statement "A", "B" & "C" below].
>
> > But statement "C" macro "DBX_REGISTER_NUMBER" gets undefined by statement
> > "D" hence the GCC hard register number gets emitted in the debug info
> > instead of DWARF register number. Previously, without this patch for SPE
> > high registers the GCC hard register number was same as the DWARF register
> > number (1200 onwards), hence we didn't see this issue.
> >
> > Removing statement "D" from "sysv4.h" file so as to generate expected
> > DWARF register number does work, but will there be any side effects?
>
> Ah, I had completely forgotten about this issue, sorry ...
>
> The problem with DBX_REGISTER_NUMBER is actually described in detail here:
> https://gcc.gnu.org/ml/gcc-patches/2012-11/msg02136.html
>
> At the time, we decided to not remove the #undef DBX_REGISTER_NUMBER to
> avoid compatibility issues, but use GCC internal numbers in .debug_frame
> and .debug_info on Linux (option (3) in the above mail). However, this
> was never actually implemented.
>
> Looking at the current status, there are three groups of rs6000 targets:
>
> - Some use the DBX_REGISTER_NUMBER definition from rs6000.h:
> These are only AIX and Darwin.
>
> - Some provide their own definition of DBX_REGISTER_NUMBER after the rs6000.h
> one was undefined by sysv4.h:
> These are FreeBSD, NetBSD, and Lynx.
>
> - All other targets do not have DBX_REGISTER_NUMBER because it is undefined
> by sysv4.h, and therefore using GCC internal register numbers:
> These are Linux, rtems, vxworks, and all ELF/EABI targets.
>
>
> The following patch tries to remove the unfortunate confusion about undefining
> and redefining DBX_REGISTER_NUMBER, while keeping the behavior on all targets
> unchanged with the following two exceptions:
> - fix the SPE problem by always translating high register numbers
> - implement option (3) above by not replacing CR2 with CR in .debug_frame
> on targets that do not use the standard DWARF register numbering
>
> The way this works is to have a common, simple implementation of
> DBX_REGISTER_NUMBER and DWARF2_FRAME_REG_OUT for all targets that just
> calls to the rs6000_dbx_register_number routine, passing an extra format
> argument that decides whether the register number is to be used for
> .debug_info, .debug_frame, or .eh_frame. In order to ensure
> rs6000_dbx_register_number always gets a GCC internal number as input,
> DWARF_FRAME_REGNUM has to be again defined as identity map.
>
> All the logic to decide debug register numbers is now contained in that
> single place. However, in order to maintain current behavior, we still
> have to distinguish between platforms that want to use the standard
> DWARF register numbering scheme, and those that use GCC internal numbers.
> This is now simply done by having the former provide a new define
> RS6000_USE_DWARF_NUMBERING in a target header file.
>
> Tested on powerpc64le-linux and powerpc64-linux.
>
> Rohit, could you verify whether this fixes the SPE problem?

Thanks for your work, I have applied your change to my 4.9 setup, rebuilt
the compiler and smoke-tested it with gdb.base/store.exp and my e500v2
multilib only; powerpc-linux-gnu target. Unfortunately your patch hasn't
changed anything, the same failures remain.

Just to be sure I haven't missed anything I reverted your change and
Rohit's one as well, rebuilt the compiler and rerun this testing and this
time all failures were gone. So it looks like your fix didn't completely
cover the damage Rohit's change caused. :(

Maciej

[r***@freescale.com]

> -----Original Message-----
> From: Maciej W. Rozycki [mailto:***@codesourcery.com]
> To: Ulrich Weigand
> Cc: Dharmakan Rohit-B30502; Wienskoski Edmar-RA8797; David Edelsohn; gcc-
> ***@gcc.gnu.org; Alan Modra; Jakub Jelinek
> Subject: Re: [RFC: Patch, PR 60102] [4.9/4.10 Regression] powerpc fp-bit
> ***@dwf_regno
[snip]
> >
> > Rohit, could you verify whether this fixes the SPE problem?
>
> Thanks for your work, I have applied your change to my 4.9 setup, rebuilt the
> compiler and smoke-tested it with gdb.base/store.exp and my e500v2 multilib
> only; powerpc-linux-gnu target. Unfortunately your patch hasn't changed
> anything, the same failures remain.
>
> Just to be sure I haven't missed anything I reverted your change and Rohit's
> one as well, rebuilt the compiler and rerun this testing and this time all failures
> were gone. So it looks like your fix didn't completely cover the damage Rohit's
> change caused. :(

Ulrich/Maciej,

The patch works for me.
Tested with GCC v4.9 branch rev 216036 and GCC trunk rev 216027.

Regards,
Rohit

[Ulrich Weigand]

rohitarulraj wrote:
> > -----Original Message-----
> > From: Maciej W. Rozycki [mailto:***@codesourcery.com]
> > To: Ulrich Weigand
> > Cc: Dharmakan Rohit-B30502; Wienskoski Edmar-RA8797; David Edelsohn; gcc-
> > ***@gcc.gnu.org; Alan Modra; Jakub Jelinek
> > Subject: Re: [RFC: Patch, PR 60102] [4.9/4.10 Regression] powerpc fp-bit
> > ***@dwf_regno
> [snip]
> > >
> > > Rohit, could you verify whether this fixes the SPE problem?
> >=20
> > Thanks for your work, I have applied your change to my 4.9 setup, rebuil=
> t the
> > compiler and smoke-tested it with gdb.base/store.exp and my e500v2 multil=
> ib
> > only; powerpc-linux-gnu target. Unfortunately your patch hasn't changed
> > anything, the same failures remain.
> >=20
> > Just to be sure I haven't missed anything I reverted your change and Roh=
> it's
> > one as well, rebuilt the compiler and rerun this testing and this time al=
> l failures
> > were gone. So it looks like your fix didn't completely cover the damage =
> Rohit's
> > change caused. :(
>
> Ulrich/Maciej,
>
> The patch works for me.
> Tested with GCC v4.9 branch rev 216036 and GCC trunk rev 216027.

Thanks for testing! Can you work with Maciej to find out why he's
seeing different results?

Bye,
Ulrich

--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
***@de.ibm.com

[Maciej W. Rozycki]

On Thu, 9 Oct 2014, Ulrich Weigand wrote:

> > The patch works for me.
> > Tested with GCC v4.9 branch rev 216036 and GCC trunk rev 216027.
>
> Thanks for testing! Can you work with Maciej to find out why he's
> seeing different results?

Seeing Rohit got good results it has struck me that perhaps one of the
patches I had previously reverted, to be able to compile GCC in the first
place, interfered with this fix -- I backed out all the subsequent patches
to test yours and Rohit's by themselves only. And it was actually the
case, with this change:

2013-05-21 Christian Bruel <***@st.com>

* dwarf2out.c (multiple_reg_loc_descriptor): Use dbx_reg_number for
spanning registers. LEAF_REG_REMAP is supported only for contiguous
registers. Set register size out of the PARALLEL loop.

back in place, in addition to your fix, I get an all-passed score for
gdb.base/store.exp. So your change looks good and my decision to back out
the other patches unfortunate. I'll yet run full e500v2 testing now to
double check, and let you know what the results are, within a couple of
hours if things work well.

Testing with my other multilibs will have to wait a few days as our Power
board farm is currently in maintenance and some are offline. Given this
situation and that you both already tested some other multilibs I think
there is little point in waiting for my full results. If anything pops up
there, then it can be addressed later on.

Thanks for your effort and sorry about the confusion with testing.

Maciej