| Message ID | 20130715191359.GQ2475@laptop.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Mon, 2013-07-15 at 21:13 +0200, Jakub Jelinek wrote: > > Looking at the patch, you've changed FIRST_PSEUDO_REGISTER (internal gcc > > thing, so fine), but that also changed DWARF_FRAME_REGISTER which was > > computed based on FIRST_PSEUDO_REGISTER. Isn't that an ABI break in the > > unwinder (think about having parts of unwinder linked from different > > versions of libgcc)? > > Can't we just tweak DWARF_FRAME_REGISTER not to include those registers? Thanks for noticing this. > I'd say something like (but, untested and can't test it right now (and no > access to power8 anyway)): Do we also need to update DWARF_REG_TO_UNWIND_COLUMN similarly, which also uses FIRST_PSEUDO_REGISTER when the dwarf reg is an SPE synthetic register? Or should we just break the dependence on FIRST_PSEUDO_REGISTER altogether so this type of thing doesn't happen again? Otherwise, we should place a comment at the definition of FIRST_PSEUDO_REGISTER warning of it's use in DWARF_FRAME_REGISTER. Peter
On Mon, Jul 15, 2013 at 02:46:56PM -0500, Peter Bergner wrote: > > I'd say something like (but, untested and can't test it right now (and no > > access to power8 anyway)): > > Do we also need to update DWARF_REG_TO_UNWIND_COLUMN similarly, which > also uses FIRST_PSEUDO_REGISTER when the dwarf reg is an SPE synthetic > register? Looks like it. Guess that "- 1" in there: #define DWARF_REG_TO_UNWIND_COLUMN(r) \ ((r) > 1200 ? ((r) - 1200 + FIRST_PSEUDO_REGISTER - 1) : (r)) stands for the sfp register and thus it would be "- 4". > Or should we just break the dependence on FIRST_PSEUDO_REGISTER > altogether so this type of thing doesn't happen again? Otherwise, we > should place a comment at the definition of FIRST_PSEUDO_REGISTER > warning of it's use in DWARF_FRAME_REGISTER. That is surely an option too. Or use (DWARF_FRAME_REGISTERS - 32) in the DWARF_REG_TO_UNWIND_COLUMN definition instead of the current FIRST_PSEUDO_REGISTER - 1, and perhaps make DWARF_FRAME_REGISTERS just a number (independent on FIRST_PSEUDO_REGISTER). Guess this depends on what David prefers. Jakub
--- gcc/config/rs6000/rs6000.h.jj 2013-07-15 21:08:02.000000000 +0200 +++ gcc/config/rs6000/rs6000.h 2013-07-15 21:11:06.846718216 +0200 @@ -892,15 +892,17 @@ enum data_align { align_abi, align_opt, in inline functions. Another pseudo (not included in DWARF_FRAME_REGISTERS) is soft frame - pointer, which is eventually eliminated in favor of SP or FP. */ + pointer, which is eventually eliminated in favor of SP or FP. + The 3 HTM registers aren't also included in DWARF_FRAME_REGISTERS. */ + #define FIRST_PSEUDO_REGISTER 117 /* This must be included for pre gcc 3.0 glibc compatibility. */ #define PRE_GCC3_DWARF_FRAME_REGISTERS 77 /* Add 32 dwarf columns for synthetic SPE registers. */ -#define DWARF_FRAME_REGISTERS ((FIRST_PSEUDO_REGISTER - 1) + 32) +#define DWARF_FRAME_REGISTERS ((FIRST_PSEUDO_REGISTER - 4) + 32) /* The SPE has an additional 32 synthetic registers, with DWARF debug info numbering for these registers starting at 1200. While eh_frame