Patchwork [rs6000,libitm] Enable Hardware Transactional Memory (HTM) support on Power

login
register
mail settings
Submitter Peter Bergner
Date July 16, 2013, 4:55 p.m.
Message ID <1373993700.4538.194.camel@otta>
Download mbox | patch
Permalink /patch/259470/
State New
Headers show

Comments

Peter Bergner - July 16, 2013, 4:55 p.m.
On Mon, 2013-07-15 at 22:35 +0200, Jakub Jelinek wrote:
> 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.

I like the idea of removing FIRST_PSEUDO_REGISTER from the definition
of DWARF_REG_TO_UNWIND_COLUMN.  I'll still let David decide whether
he wants DWARF_FRAME_REGISTERS to be just a number of computed off
of FIRST_PSEUDO_REGISTER.  In the meantime, I'll bootstrap and
regtest your patch along with the DWARF_REG_TO_UNWIND_COLUMN
change.

Peter

	* config/rs6000/rs6000.h (FIRST_PSEUDO_REGISTERS): Mention HTM
	registers in the comment.
	(DWARF_FRAME_REGISTERS): Subtract also the 3 HTM registers.
	(DWARF_REG_TO_UNWIND_COLUMN): Use DWARF_FRAME_REGISTERS
	rather than FIRST_PSEUDO_REGISTERS.
Peter Bergner - July 16, 2013, 7:27 p.m.
On Tue, 2013-07-16 at 11:55 -0500, Peter Bergner wrote:
> 	* config/rs6000/rs6000.h (FIRST_PSEUDO_REGISTERS): Mention HTM
> 	registers in the comment.
> 	(DWARF_FRAME_REGISTERS): Subtract also the 3 HTM registers.
> 	(DWARF_REG_TO_UNWIND_COLUMN): Use DWARF_FRAME_REGISTERS
> 	rather than FIRST_PSEUDO_REGISTERS.

FYI, this bootstrapped and regtested with no regressions.
David, is this ok for mainline?

Peter
David Edelsohn - July 16, 2013, 7:31 p.m.
On Tue, Jul 16, 2013 at 3:27 PM, Peter Bergner <bergner@vnet.ibm.com> wrote:
> On Tue, 2013-07-16 at 11:55 -0500, Peter Bergner wrote:
>>       * config/rs6000/rs6000.h (FIRST_PSEUDO_REGISTERS): Mention HTM
>>       registers in the comment.
>>       (DWARF_FRAME_REGISTERS): Subtract also the 3 HTM registers.
>>       (DWARF_REG_TO_UNWIND_COLUMN): Use DWARF_FRAME_REGISTERS
>>       rather than FIRST_PSEUDO_REGISTERS.
>
> FYI, this bootstrapped and regtested with no regressions.
> David, is this ok for mainline?

Okay.

Thanks, David
Peter Bergner - July 16, 2013, 9:07 p.m.
On Tue, 2013-07-16 at 15:31 -0400, David Edelsohn wrote:
> On Tue, Jul 16, 2013 at 3:27 PM, Peter Bergner <bergner@vnet.ibm.com> wrote:
> > On Tue, 2013-07-16 at 11:55 -0500, Peter Bergner wrote:
> >>       * config/rs6000/rs6000.h (FIRST_PSEUDO_REGISTERS): Mention HTM
> >>       registers in the comment.
> >>       (DWARF_FRAME_REGISTERS): Subtract also the 3 HTM registers.
> >>       (DWARF_REG_TO_UNWIND_COLUMN): Use DWARF_FRAME_REGISTERS
> >>       rather than FIRST_PSEUDO_REGISTERS.
> >
> > FYI, this bootstrapped and regtested with no regressions.
> > David, is this ok for mainline?
> 
> Okay.

Committed as revision 200988.  Thanks Jakub and David!

Peter

Patch

Index: gcc/config/rs6000/rs6000.h
===================================================================
--- gcc/config/rs6000/rs6000.h	(revision 200985)
+++ gcc/config/rs6000/rs6000.h	(working copy)
@@ -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
@@ -916,7 +918,7 @@  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 + FIRST_PSEUDO_REGISTER - 1) : (r))
+  ((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)