public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH, rs6000, testsuite] Fix PR target/64579, __TM_end __builtin_tend failed to return transactional state
@ 2015-03-20 19:27 Peter Bergner
  2015-03-20 20:52 ` Segher Boessenkool
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Bergner @ 2015-03-20 19:27 UTC (permalink / raw)
  To: GCC Patches; +Cc: David Edelsohn, Michael Meissner, Bill Schmidt

PR target/64579 exposes a bug in the definitions of the HTM builtins
and associated define_expands.  All of the HTM builtins were defined to
return true on success and false on failure, but that only makes sense
for the tbegin builtin (which is unchanged here).  It makes more sense
for the other builtins to just return the contents of the CR register
(usually cr0) that is updated upon execution of the HTM instruction
instead, since it contains the Transaction Status bits.  This makes them
similar to the __builtin_ttest() builtin which already does that.
I have also updated the gcc documentation to reflect these changes.

I believe this redefinition of the builtins is "safe", since for the
builtins I'm redefining, the common usage case is to just ignore the
builtin return result altogether. This is proved by the recent fix
by Adhemerval that fixed the tcheck pattern to not be a recormd form
instruction.  Had anyone actually used the builtin, the assembler would
have flagged it as invalid.

As a result of these changes, the __TM_end() XL intrinsic function which
was the topic of this bugzilla, now correctly returns the transaction
status per the XL HTM API.

I'll also note Adhemerval submitted a patch for LLVM to add HTM support
for these HTM builtins and they match the changes I have made here.
The XL compiler doesn't support our low level HTM builtins, so there's
problems there.

I have also added an executable HTM test case that tests the tbegin, tend,
tsuspend, tresume and tcheck builtins are compiled, assembled and execute
correctly.

This passed bootstrapping and regtesting with no regressions.
Is this ok for trunk (stage1 or stage4)?  I'd like to also
backport this to the release branches so that all of the branches
agree on the builtin return values.  Is that ok?

Peter


gcc/:

	PR target/64579
	* config/rs6000/htm.md (tabort, tabortdc, tabortdci,
	tabortwc, tabortwci, ttest, tcheck, tend, trechkpt,
	treclaim, tsr): Use rs6000_emit_move_from_cr_field.
	(tbegin, tabort_internal): Use gpc_reg_operand.
	(tcheck_internal): Remove operand.
	(tabort_internal, tabortdc_internal, tabortdci_internal,
	tabortwc_internal, tabortwci_internal, ttest_internal,
	tcheck_internal, tend_internal, trechkpt_internal, treclaim_internal,
	tsr_internal): Remove * prefix from existing define_insn names.
	* config/rs6000/rs6000-builtin.def (tcheck): Remove builtin argument.
	* config/rs6000/rs6000-protos.h (rs6000_emit_move_from_cr_field): New.
	* config/rs6000/rs6000.c (rs6000_emit_move_from_cr_field): Likewise.
	* config/rs6000/rs6000.h (RS6000_BTC_32BIT): Delete.
	(RS6000_BTC_64BIT): Likewise.
	(RS6000_BTC_MISC_MASK): Update.
	* config/rs6000/htmxlintrin.h (__TM_end): Use _HTM_TRANSACTIONAL as
	expected value.
	* doc/extend.texi: Update documentation for htm builtins.

gcc/testsuite/:

	PR target/64579
	* gcc.target/powerpc/htm-builtin-1.c (__builtin_tcheck): Remove operand.
	* gcc.target/powerpc/htm-1.c: New test.
	* lib/target-supports.exp (check_htm_hw_available): New function.

Index: gcc/config/rs6000/htm.md
===================================================================
--- gcc/config/rs6000/htm.md	(revision 221519)
+++ gcc/config/rs6000/htm.md	(working copy)
@@ -48,24 +48,19 @@ (define_c_enum "unspecv"
 
 
 (define_expand "tabort"
-  [(set (match_dup 2)
-	(unspec_volatile:CC [(match_operand:SI 1 "int_reg_operand" "")]
-			    UNSPECV_HTM_TABORT))
-   (set (match_dup 3)
-	(eq:SI (match_dup 2)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 3)
-		(const_int 1)))]
+  [(match_operand:SI 0 "gpc_reg_operand" "")
+   (match_operand:SI 1 "gpc_reg_operand" "")]
   "TARGET_HTM"
 {
-  operands[2] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[3] = gen_reg_rtx (SImode);
+  rtx cr = gen_reg_rtx (CCmode);
+  emit_insn (gen_tabort_internal (operands[1], cr));
+  rs6000_emit_move_from_cr_field (operands[0], cr);
+  DONE;
 })
 
-(define_insn "*tabort_internal"
+(define_insn "tabort_internal"
   [(set (match_operand:CC 1 "cc_reg_operand" "=x")
-	(unspec_volatile:CC [(match_operand:SI 0 "int_reg_operand" "r")]
+	(unspec_volatile:CC [(match_operand:SI 0 "gpc_reg_operand" "r")]
 			    UNSPECV_HTM_TABORT))]
   "TARGET_HTM"
   "tabort. %0"
@@ -73,24 +68,19 @@ (define_insn "*tabort_internal"
    (set_attr "length" "4")])
 
 (define_expand "tabortdc"
-  [(set (match_dup 4)
-	(unspec_volatile:CC [(match_operand 1 "u5bit_cint_operand" "n")
-			     (match_operand:SI 2 "gpc_reg_operand" "r")
-			     (match_operand:SI 3 "gpc_reg_operand" "r")]
-			    UNSPECV_HTM_TABORTDC))
-   (set (match_dup 5)
-	(eq:SI (match_dup 4)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 5)
-		(const_int 1)))]
+  [(match_operand:SI 0 "gpc_reg_operand" "")
+   (match_operand 1 "u5bit_cint_operand" "n")
+   (match_operand:SI 2 "gpc_reg_operand" "")
+   (match_operand:SI 3 "gpc_reg_operand" "")]
   "TARGET_HTM"
 {
-  operands[4] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[5] = gen_reg_rtx (SImode);
+  rtx cr = gen_reg_rtx (CCmode);
+  emit_insn (gen_tabortdc_internal (operands[1], operands[2], operands[3], cr));
+  rs6000_emit_move_from_cr_field (operands[0], cr);
+  DONE;
 })
 
-(define_insn "*tabortdc_internal"
+(define_insn "tabortdc_internal"
   [(set (match_operand:CC 3 "cc_reg_operand" "=x")
 	(unspec_volatile:CC [(match_operand 0 "u5bit_cint_operand" "n")
 			     (match_operand:SI 1 "gpc_reg_operand" "r")
@@ -102,24 +92,19 @@ (define_insn "*tabortdc_internal"
    (set_attr "length" "4")])
 
 (define_expand "tabortdci"
-  [(set (match_dup 4)
-	(unspec_volatile:CC [(match_operand 1 "u5bit_cint_operand" "n")
-			     (match_operand:SI 2 "gpc_reg_operand" "r")
-			     (match_operand 3 "s5bit_cint_operand" "n")]
-			    UNSPECV_HTM_TABORTDCI))
-   (set (match_dup 5)
-	(eq:SI (match_dup 4)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 5)
-		(const_int 1)))]
+  [(match_operand:SI 0 "gpc_reg_operand" "")
+   (match_operand 1 "u5bit_cint_operand" "n")
+   (match_operand:SI 2 "gpc_reg_operand" "")
+   (match_operand 3 "s5bit_cint_operand" "n")]
   "TARGET_HTM"
 {
-  operands[4] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[5] = gen_reg_rtx (SImode);
+  rtx cr = gen_reg_rtx (CCmode);
+  emit_insn (gen_tabortdci_internal (operands[1], operands[2], operands[3], cr));
+  rs6000_emit_move_from_cr_field (operands[0], cr);
+  DONE;
 })
 
-(define_insn "*tabortdci_internal"
+(define_insn "tabortdci_internal"
   [(set (match_operand:CC 3 "cc_reg_operand" "=x")
 	(unspec_volatile:CC [(match_operand 0 "u5bit_cint_operand" "n")
 			     (match_operand:SI 1 "gpc_reg_operand" "r")
@@ -131,24 +116,19 @@ (define_insn "*tabortdci_internal"
    (set_attr "length" "4")])
 
 (define_expand "tabortwc"
-  [(set (match_dup 4)
-	(unspec_volatile:CC [(match_operand 1 "u5bit_cint_operand" "n")
-			     (match_operand:SI 2 "gpc_reg_operand" "r")
-			     (match_operand:SI 3 "gpc_reg_operand" "r")]
-			    UNSPECV_HTM_TABORTWC))
-   (set (match_dup 5)
-	(eq:SI (match_dup 4)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 5)
-		(const_int 1)))]
+  [(match_operand:SI 0 "gpc_reg_operand" "")
+   (match_operand 1 "u5bit_cint_operand" "n")
+   (match_operand:SI 2 "gpc_reg_operand" "")
+   (match_operand:SI 3 "gpc_reg_operand" "")]
   "TARGET_HTM"
 {
-  operands[4] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[5] = gen_reg_rtx (SImode);
+  rtx cr = gen_reg_rtx (CCmode);
+  emit_insn (gen_tabortwc_internal (operands[1], operands[2], operands[3], cr));
+  rs6000_emit_move_from_cr_field (operands[0], cr);
+  DONE;
 })
 
-(define_insn "*tabortwc_internal"
+(define_insn "tabortwc_internal"
   [(set (match_operand:CC 3 "cc_reg_operand" "=x")
 	(unspec_volatile:CC [(match_operand 0 "u5bit_cint_operand" "n")
 			     (match_operand:SI 1 "gpc_reg_operand" "r")
@@ -160,42 +140,30 @@ (define_insn "*tabortwc_internal"
    (set_attr "length" "4")])
 
 (define_expand "tabortwci"
-  [(set (match_dup 4)
-	(unspec_volatile:CC [(match_operand 1 "u5bit_cint_operand" "n")
-			     (match_operand:SI 2 "gpc_reg_operand" "r")
-			     (match_operand 3 "s5bit_cint_operand" "n")]
-			    UNSPECV_HTM_TABORTWCI))
-   (set (match_dup 5)
-	(eq:SI (match_dup 4)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 5)
-		(const_int 1)))]
+  [(match_operand:SI 0 "gpc_reg_operand" "")
+   (match_operand 1 "u5bit_cint_operand" "n")
+   (match_operand:SI 2 "gpc_reg_operand" "")
+   (match_operand 3 "s5bit_cint_operand" "n")]
   "TARGET_HTM"
 {
-  operands[4] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[5] = gen_reg_rtx (SImode);
+  rtx cr = gen_reg_rtx (CCmode);
+  emit_insn (gen_tabortwci_internal (operands[1], operands[2], operands[3], cr));
+  rs6000_emit_move_from_cr_field (operands[0], cr);
+  DONE;
 })
 
 (define_expand "ttest"
-  [(set (match_dup 1)
-	(unspec_volatile:CC [(const_int 0)
-			     (reg:SI 0)
-			     (const_int 0)]
-			    UNSPECV_HTM_TABORTWCI))
-   (set (subreg:CC (match_dup 2) 0) (match_dup 1))
-   (set (match_dup 3) (lshiftrt:SI (match_dup 2) (const_int 28)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(and:SI (match_dup 3)
-		(const_int 15)))]
+  [(match_operand:SI 0 "gpc_reg_operand" "")]
   "TARGET_HTM"
 {
-  operands[1] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[2] = gen_reg_rtx (SImode);
-  operands[3] = gen_reg_rtx (SImode);
+  rtx cr = gen_reg_rtx (CCmode);
+  rtx r0 = gen_rtx_REG (SImode, FIRST_GPR_REGNO);
+  emit_insn (gen_tabortwci_internal (GEN_INT (0), r0, GEN_INT (0), cr));
+  rs6000_emit_move_from_cr_field (operands[0], cr);
+  DONE;
 })
 
-(define_insn "*tabortwci_internal"
+(define_insn "tabortwci_internal"
   [(set (match_operand:CC 3 "cc_reg_operand" "=x")
 	(unspec_volatile:CC [(match_operand 0 "u5bit_cint_operand" "n")
 			     (match_operand:SI 1 "gpc_reg_operand" "r")
@@ -213,7 +181,7 @@ (define_expand "tbegin"
    (set (match_dup 3)
 	(eq:SI (match_dup 2)
 	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
+   (set (match_operand:SI 0 "gpc_reg_operand" "")
 	(xor:SI (match_dup 3)
 		(const_int 1)))]
   "TARGET_HTM"
@@ -232,24 +200,18 @@ (define_insn "*tbegin_internal"
    (set_attr "length" "4")])
 
 (define_expand "tcheck"
-  [(set (match_dup 2)
-	(unspec_volatile:CC [(match_operand 1 "u3bit_cint_operand" "n")]
-			    UNSPECV_HTM_TCHECK))
-   (set (match_dup 3)
-	(eq:SI (match_dup 2)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 3)
-		(const_int 1)))]
+  [(match_operand:SI 0 "gpc_reg_operand" "")]
   "TARGET_HTM"
 {
-  operands[2] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[3] = gen_reg_rtx (SImode);
+  rtx cr = gen_reg_rtx (CCmode);
+  emit_insn (gen_tcheck_internal (cr));
+  rs6000_emit_move_from_cr_field (operands[0], cr);
+  DONE;
 })
 
-(define_insn "*tcheck_internal"
-  [(set (match_operand:CC 1 "cc_reg_operand" "=x")
-	(unspec_volatile:CC [(match_operand 0 "u3bit_cint_operand" "n")]
+(define_insn "tcheck_internal"
+  [(set (match_operand:CC 0 "cc_reg_operand" "=y")
+	(unspec_volatile:CC [(const_int 0)]
 			    UNSPECV_HTM_TCHECK))]
   "TARGET_HTM"
   "tcheck %0"
@@ -257,22 +219,17 @@ (define_insn "*tcheck_internal"
    (set_attr "length" "4")])
 
 (define_expand "tend"
-  [(set (match_dup 2)
-	(unspec_volatile:CC [(match_operand 1 "const_0_to_1_operand" "n")]
-			    UNSPECV_HTM_TEND))
-   (set (match_dup 3)
-	(eq:SI (match_dup 2)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 3)
-		(const_int 1)))]
+  [(match_operand:SI 0 "gpc_reg_operand" "")
+   (match_operand 1 "const_0_to_1_operand" "n")]
   "TARGET_HTM"
 {
-  operands[2] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[3] = gen_reg_rtx (SImode);
+  rtx cr = gen_reg_rtx (CCmode);
+  emit_insn (gen_tend_internal (operands[1], cr));
+  rs6000_emit_move_from_cr_field (operands[0], cr);
+  DONE;
 })
 
-(define_insn "*tend_internal"
+(define_insn "tend_internal"
   [(set (match_operand:CC 1 "cc_reg_operand" "=x")
 	(unspec_volatile:CC [(match_operand 0 "const_0_to_1_operand" "n")]
 			    UNSPECV_HTM_TEND))]
@@ -282,22 +239,16 @@ (define_insn "*tend_internal"
    (set_attr "length" "4")])
 
 (define_expand "trechkpt"
-  [(set (match_dup 1)
-	(unspec_volatile:CC [(const_int 0)]
-			    UNSPECV_HTM_TRECHKPT))
-   (set (match_dup 2)
-	(eq:SI (match_dup 1)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 2)
-		(const_int 1)))]
+  [(match_operand:SI 0 "gpc_reg_operand" "")]
   "TARGET_HTM"
 {
-  operands[1] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[2] = gen_reg_rtx (SImode);
+  rtx cr = gen_reg_rtx (CCmode);
+  emit_insn (gen_trechkpt_internal (cr));
+  rs6000_emit_move_from_cr_field (operands[0], cr);
+  DONE;
 })
 
-(define_insn "*trechkpt_internal"
+(define_insn "trechkpt_internal"
   [(set (match_operand:CC 0 "cc_reg_operand" "=x")
 	(unspec_volatile:CC [(const_int 0)]
 			    UNSPECV_HTM_TRECHKPT))]
@@ -307,22 +258,17 @@ (define_insn "*trechkpt_internal"
    (set_attr "length" "4")])
 
 (define_expand "treclaim"
-  [(set (match_dup 2)
-	(unspec_volatile:CC [(match_operand:SI 1 "gpc_reg_operand" "r")]
-			    UNSPECV_HTM_TRECLAIM))
-   (set (match_dup 3)
-	(eq:SI (match_dup 2)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 3)
-		(const_int 1)))]
+  [(match_operand:SI 0 "gpc_reg_operand" "")
+   (match_operand:SI 1 "gpc_reg_operand" "")]
   "TARGET_HTM"
 {
-  operands[2] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[3] = gen_reg_rtx (SImode);
+  rtx cr = gen_reg_rtx (CCmode);
+  emit_insn (gen_treclaim_internal (operands[1], cr));
+  rs6000_emit_move_from_cr_field (operands[0], cr);
+  DONE;
 })
 
-(define_insn "*treclaim_internal"
+(define_insn "treclaim_internal"
   [(set (match_operand:CC 1 "cc_reg_operand" "=x")
 	(unspec_volatile:CC [(match_operand:SI 0 "gpc_reg_operand" "r")]
 			    UNSPECV_HTM_TRECLAIM))]
@@ -332,22 +278,17 @@ (define_insn "*treclaim_internal"
    (set_attr "length" "4")])
 
 (define_expand "tsr"
-  [(set (match_dup 2)
-	(unspec_volatile:CC [(match_operand 1 "const_0_to_1_operand" "n")]
-			    UNSPECV_HTM_TSR))
-   (set (match_dup 3)
-	(eq:SI (match_dup 2)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 3)
-		(const_int 1)))]
+  [(match_operand:SI 0 "gpc_reg_operand" "")
+   (match_operand 1 "const_0_to_1_operand" "n")]
   "TARGET_HTM"
 {
-  operands[2] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[3] = gen_reg_rtx (SImode);
+  rtx cr = gen_reg_rtx (CCmode);
+  emit_insn (gen_tsr_internal (operands[1], cr));
+  rs6000_emit_move_from_cr_field (operands[0], cr);
+  DONE;
 })
 
-(define_insn "*tsr_internal"
+(define_insn "tsr_internal"
   [(set (match_operand:CC 1 "cc_reg_operand" "=x")
 	(unspec_volatile:CC [(match_operand 0 "const_0_to_1_operand" "n")]
 			    UNSPECV_HTM_TSR))]
Index: gcc/config/rs6000/rs6000-builtin.def
===================================================================
--- gcc/config/rs6000/rs6000-builtin.def	(revision 221519)
+++ gcc/config/rs6000/rs6000-builtin.def	(working copy)
@@ -1639,7 +1639,7 @@ BU_HTM_3  (TABORTDCI,	"tabortdci",	MISC,
 BU_HTM_3  (TABORTWC,	"tabortwc",	MISC,	tabortwc)
 BU_HTM_3  (TABORTWCI,	"tabortwci",	MISC,	tabortwci)
 BU_HTM_1  (TBEGIN,	"tbegin",	MISC,	tbegin)
-BU_HTM_1  (TCHECK,	"tcheck",	MISC,	tcheck)
+BU_HTM_0  (TCHECK,	"tcheck",	MISC,	tcheck)
 BU_HTM_1  (TEND,	"tend",		MISC,	tend)
 BU_HTM_0  (TENDALL,	"tendall",	MISC,	tend)
 BU_HTM_0  (TRECHKPT,	"trechkpt",	MISC,	trechkpt)
Index: gcc/config/rs6000/rs6000-protos.h
===================================================================
--- gcc/config/rs6000/rs6000-protos.h	(revision 221519)
+++ gcc/config/rs6000/rs6000-protos.h	(working copy)
@@ -132,6 +132,7 @@ extern rtx create_TOC_reference (rtx, rt
 extern void rs6000_split_multireg_move (rtx, rtx);
 extern void rs6000_emit_le_vsx_move (rtx, rtx, machine_mode);
 extern void rs6000_emit_move (rtx, rtx, machine_mode);
+extern void rs6000_emit_move_from_cr_field (rtx, rtx);
 extern rtx rs6000_secondary_memory_needed_rtx (machine_mode);
 extern machine_mode rs6000_secondary_memory_needed_mode (enum
 							      machine_mode);
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 221519)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -23320,6 +23320,22 @@ rs6000_emit_move_from_cr (rtx reg)
   emit_insn (gen_movesi_from_cr (reg));
 }
 
+/* Emit code to copy one 4-bit condition register field into the
+   least significant end of register DEST.  */
+
+void
+rs6000_emit_move_from_cr_field (rtx dest, rtx cr)
+{
+  machine_mode mode = GET_MODE (dest);
+  rtx scratch1 = gen_reg_rtx (mode);
+  rtx scratch2 = gen_reg_rtx (mode);
+  rtx subreg = simplify_gen_subreg (CCmode, scratch1, mode, 0);
+
+  emit_insn (gen_movcc (subreg, cr));
+  emit_insn (gen_lshrsi3 (scratch2, scratch1, GEN_INT (28)));
+  emit_insn (gen_andsi3 (dest, scratch2, GEN_INT (0xf)));
+}
+
 /* Determine whether the gp REG is really used.  */
 
 static bool
Index: gcc/config/rs6000/rs6000.h
===================================================================
--- gcc/config/rs6000/rs6000.h	(revision 221519)
+++ gcc/config/rs6000/rs6000.h	(working copy)
@@ -2574,9 +2574,7 @@ extern int frame_pointer_needed;
 #define RS6000_BTC_SPR		0x01000000	/* function references SPRs.  */
 #define RS6000_BTC_VOID		0x02000000	/* function has no return value.  */
 #define RS6000_BTC_OVERLOADED	0x04000000	/* function is overloaded.  */
-#define RS6000_BTC_32BIT	0x08000000	/* function references SPRs.  */
-#define RS6000_BTC_64BIT	0x10000000	/* function references SPRs.  */
-#define RS6000_BTC_MISC_MASK	0x1f000000	/* Mask of the misc info.  */
+#define RS6000_BTC_MISC_MASK	0x07000000	/* Mask of the misc info.  */
 
 /* Convenience macros to document the instruction type.  */
 #define RS6000_BTC_MEM		RS6000_BTC_MISC	/* load/store touches mem.  */
Index: htmxlintrin.h
===================================================================
--- htmxlintrin.h	(revision 221519)
+++ htmxlintrin.h	(working copy)
@@ -81,7 +81,8 @@ extern __inline long
 __attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
 __TM_end (void)
 {
-  if (__builtin_expect (__builtin_tend (0), 1))
+  unsigned char status = _HTM_STATE (__builtin_tend (0));
+  if (__builtin_expect (status, _HTM_TRANSACTIONAL))
     return 1;
   return 0;
 }
Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 221519)
+++ gcc/doc/extend.texi	(working copy)
@@ -15000,10 +15000,15 @@ The following low level built-in functio
 @option{-mhtm} or @option{-mcpu=CPU} where CPU is `power8' or later.
 They all generate the machine instruction that is part of the name.
 
-The HTM built-ins return true or false depending on their success and
-their arguments match exactly the type and order of the associated
-hardware instruction's operands.  Refer to the ISA manual for a
-description of each instruction's operands.
+The HTM builtins (with the exception of @code{__builtin_tbegin}) return
+the full 4-bit condition register value set by their associated hardware
+instruction.  The header file @code{htmintrin.h} defines some macros that can
+be used to decipher the return value.  The @code{__builtin_tbegin} builtin
+returns a simple true or false value depending on whether a transaction was
+successfully started or not.  The arguments of the builtins match exactly the
+type and order of the associated hardware instruction's operands, except for
+the @code{__builtin_tcheck} builtin, which does not take any input arguments.
+Refer to the ISA manual for a description of each instruction's operands.
 
 @smallexample
 unsigned int __builtin_tbegin (unsigned int)
@@ -15015,7 +15020,7 @@ unsigned int __builtin_tabortdci (unsign
 unsigned int __builtin_tabortwc (unsigned int, unsigned int, unsigned int)
 unsigned int __builtin_tabortwci (unsigned int, unsigned int, int)
 
-unsigned int __builtin_tcheck (unsigned int)
+unsigned int __builtin_tcheck (void)
 unsigned int __builtin_treclaim (unsigned int)
 unsigned int __builtin_trechkpt (void)
 unsigned int __builtin_tsr (unsigned int)
@@ -15150,7 +15155,7 @@ TM_buff_type TM_buff;
 
 while (1)
   @{
-    if (__TM_begin (TM_buff))
+    if (__TM_begin (TM_buff) == _HTM_TBEGIN_STARTED)
       @{
         /* Transaction State Initiated.  */
         if (is_locked (lock))
Index: gcc/testsuite/gcc.target/powerpc/htm-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/htm-1.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/htm-1.c	(working copy)
@@ -0,0 +1,53 @@
+/* { dg-do run { target { powerpc*-*-* && htm_hw } } } */
+/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */
+/* { dg-require-effective-target powerpc_htm_ok } */
+/* { dg-options "-mhtm" } */
+
+/* Program to test PowerPC HTM instructions.  */
+
+#include <stdlib.h>
+#include <htmintrin.h>
+
+int
+main (void)
+{
+  long i;
+  unsigned long mask = 0;
+
+repeat:
+  if (__builtin_tbegin (0))
+    {
+      mask++;
+    }
+  else
+    abort();
+
+  if (mask == 1)
+    {
+      __builtin_tsuspend ();
+
+      if (_HTM_STATE (__builtin_tcheck ()) != _HTM_SUSPENDED)
+	abort ();
+
+      __builtin_tresume ();
+
+      if (_HTM_STATE (__builtin_tcheck ()) != _HTM_TRANSACTIONAL)
+	abort ();
+    }
+  else
+    mask++;
+
+  if (_HTM_STATE (__builtin_tendall ()) != _HTM_TRANSACTIONAL)
+    abort ();
+
+  if (mask == 1)
+    goto repeat;
+
+  if (_HTM_STATE (__builtin_tendall ()) != _HTM_NONTRANSACTIONAL)
+    abort ();
+
+  if (mask != 3)
+    abort ();
+
+  return 0;
+}
Index: gcc/testsuite/gcc.target/powerpc/htm-builtin-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/htm-builtin-1.c	(revision 221519)
+++ gcc/testsuite/gcc.target/powerpc/htm-builtin-1.c	(working copy)
@@ -30,7 +30,7 @@ void use_builtins (long *p, char code, l
   p[7] = __builtin_tabortwc (0xf, a[7], b[7]);
   p[8] = __builtin_tabortwci (0xf, a[8], 13);
 
-  p[9] = __builtin_tcheck (5);
+  p[9] = __builtin_tcheck ();
   p[10] = __builtin_trechkpt ();
   p[11] = __builtin_treclaim (0);
   p[12] = __builtin_tresume ();
Index: gcc/testsuite/lib/target-supports.exp
===================================================================
--- gcc/testsuite/lib/target-supports.exp	(revision 221519)
+++ gcc/testsuite/lib/target-supports.exp	(working copy)
@@ -3251,6 +3251,25 @@ proc check_effective_target_powerpc_htm_
     }
 }
 
+# Return 1 if the target supports executing HTM hardware instructions,
+# 0 otherwise.  Cache the result.
+
+proc check_htm_hw_available { } {
+    return [check_cached_effective_target htm_hw_available {
+	# For now, disable on Darwin
+	if { [istarget powerpc-*-eabi] || [istarget powerpc*-*-eabispe] || [istarget *-*-darwin*]} {
+	    expr 0
+	} else {
+	    check_runtime_nocache htm_hw_available {
+		int main()
+		{
+		  __builtin_ttest ();
+		  return 0;
+		}
+	    } "-mhtm"
+	}
+    }]
+}
 # Return 1 if this is a PowerPC target supporting -mcpu=cell.
 
 proc check_effective_target_powerpc_ppu_ok { } {
@@ -5250,6 +5269,7 @@ proc is-effective-target { arg } {
 	  "p8vector_hw"    { set selected [check_p8vector_hw_available] }
 	  "ppc_recip_hw"   { set selected [check_ppc_recip_hw_available] }
 	  "dfp_hw"         { set selected [check_dfp_hw_available] }
+	  "htm_hw"         { set selected [check_htm_hw_available] }
 	  "named_sections" { set selected [check_named_sections_available] }
 	  "gc_sections"    { set selected [check_gc_sections_available] }
 	  "cxa_atexit"     { set selected [check_cxa_atexit_available] }
@@ -5273,6 +5293,7 @@ proc is-effective-target-keyword { arg }
 	  "p8vector_hw"    { return 1 }
 	  "ppc_recip_hw"   { return 1 }
 	  "dfp_hw"         { return 1 }
+	  "htm_hw"         { return 1 }
 	  "named_sections" { return 1 }
 	  "gc_sections"    { return 1 }
 	  "cxa_atexit"     { return 1 }


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH, rs6000, testsuite] Fix PR target/64579, __TM_end __builtin_tend failed to return transactional state
  2015-03-20 19:27 [PATCH, rs6000, testsuite] Fix PR target/64579, __TM_end __builtin_tend failed to return transactional state Peter Bergner
@ 2015-03-20 20:52 ` Segher Boessenkool
  2015-03-20 22:42   ` Peter Bergner
  0 siblings, 1 reply; 14+ messages in thread
From: Segher Boessenkool @ 2015-03-20 20:52 UTC (permalink / raw)
  To: Peter Bergner; +Cc: GCC Patches, David Edelsohn, Michael Meissner, Bill Schmidt

Hi Peter,

Nice patch.  Some minor things...


On Fri, Mar 20, 2015 at 02:27:40PM -0500, Peter Bergner wrote:
> Index: gcc/config/rs6000/htm.md
> ===================================================================
> --- gcc/config/rs6000/htm.md	(revision 221519)
> +++ gcc/config/rs6000/htm.md	(working copy)
> @@ -48,24 +48,19 @@ (define_c_enum "unspecv"
>  
>  
>  (define_expand "tabort"
> -  [(set (match_dup 2)
> -	(unspec_volatile:CC [(match_operand:SI 1 "int_reg_operand" "")]
> -			    UNSPECV_HTM_TABORT))
> -   (set (match_dup 3)
> -	(eq:SI (match_dup 2)
> -	       (const_int 0)))
> -   (set (match_operand:SI 0 "int_reg_operand" "")
> -	(xor:SI (match_dup 3)
> -		(const_int 1)))]
> +  [(match_operand:SI 0 "gpc_reg_operand" "")
> +   (match_operand:SI 1 "gpc_reg_operand" "")]
>    "TARGET_HTM"
>  {
> -  operands[2] = gen_rtx_REG (CCmode, CR0_REGNO);
> -  operands[3] = gen_reg_rtx (SImode);
> +  rtx cr = gen_reg_rtx (CCmode);
> +  emit_insn (gen_tabort_internal (operands[1], cr));
> +  rs6000_emit_move_from_cr_field (operands[0], cr);
> +  DONE;
>  })

Maybe it would be nicer if the builtin-expansion code handled the copy
from cc, instead of stacking on RTL expanders.

>  (define_expand "tabortdc"
> -  [(set (match_dup 4)
> -	(unspec_volatile:CC [(match_operand 1 "u5bit_cint_operand" "n")
> -			     (match_operand:SI 2 "gpc_reg_operand" "r")
> -			     (match_operand:SI 3 "gpc_reg_operand" "r")]
> -			    UNSPECV_HTM_TABORTDC))
> -   (set (match_dup 5)
> -	(eq:SI (match_dup 4)
> -	       (const_int 0)))
> -   (set (match_operand:SI 0 "int_reg_operand" "")
> -	(xor:SI (match_dup 5)
> -		(const_int 1)))]
> +  [(match_operand:SI 0 "gpc_reg_operand" "")
> +   (match_operand 1 "u5bit_cint_operand" "n")
> +   (match_operand:SI 2 "gpc_reg_operand" "")
> +   (match_operand:SI 3 "gpc_reg_operand" "")]
>    "TARGET_HTM"
>  {
> -  operands[4] = gen_rtx_REG (CCmode, CR0_REGNO);
> -  operands[5] = gen_reg_rtx (SImode);
> +  rtx cr = gen_reg_rtx (CCmode);
> +  emit_insn (gen_tabortdc_internal (operands[1], operands[2], operands[3], cr));
> +  rs6000_emit_move_from_cr_field (operands[0], cr);
> +  DONE;
>  })

Expanders have no constraints (you can leave out the field completely).
Doesn't gen* warn on non-empty constraints?

> --- gcc/testsuite/gcc.target/powerpc/htm-1.c	(revision 0)
> +++ gcc/testsuite/gcc.target/powerpc/htm-1.c	(working copy)
> @@ -0,0 +1,53 @@
> +/* { dg-do run { target { powerpc*-*-* && htm_hw } } } */
> +/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */

htm_hw already disallows Darwin?  [ And {"*"} {""} is default. ]

> +	# For now, disable on Darwin
> +	if { [istarget powerpc-*-eabi] || [istarget powerpc*-*-eabispe] || [istarget *-*-darwin*]} {
> +	    expr 0

If ever HTM is enabled on Darwin, the testcases should Just Work as far
as I see.


Cheers,


Segher

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH, rs6000, testsuite] Fix PR target/64579, __TM_end __builtin_tend failed to return transactional state
  2015-03-20 20:52 ` Segher Boessenkool
@ 2015-03-20 22:42   ` Peter Bergner
  2015-04-21 20:56     ` Peter Bergner
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Bergner @ 2015-03-20 22:42 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: GCC Patches, David Edelsohn, Michael Meissner, Bill Schmidt

On Fri, 2015-03-20 at 15:52 -0500, Segher Boessenkool wrote:
> Maybe it would be nicer if the builtin-expansion code handled the copy
> from cc, instead of stacking on RTL expanders.

That would allow getting rid of the expanders completely, which
would be nice.  I'd have to somehow add some type of RS6000_BTC_*
flag onto the builtin though, so I can tell during builtin expansion
whether I need to emit the cr copy code or not.  I'll give that a
try ...when I return from sunny Mexico in a week. :-)
Thanks for the suggestion.



> >  (define_expand "tabortdc"
[snip]
> > +   (match_operand 1 "u5bit_cint_operand" "n")
[snip]
> 
> Expanders have no constraints (you can leave out the field completely).
> Doesn't gen* warn on non-empty constraints?

Correct, and David mentioned this when I first submitted the original
HTM patch, but I replied they were added to allow better error
messages when people used out of range integers for builtin args:

  https://gcc.gnu.org/ml/gcc-patches/2013-07/msg00565.html



> > --- gcc/testsuite/gcc.target/powerpc/htm-1.c	(revision 0)
> > +++ gcc/testsuite/gcc.target/powerpc/htm-1.c	(working copy)
> > @@ -0,0 +1,53 @@
> > +/* { dg-do run { target { powerpc*-*-* && htm_hw } } } */
> > +/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */
> 
> htm_hw already disallows Darwin?  [ And {"*"} {""} is default. ]
> 
> > +	# For now, disable on Darwin
> > +	if { [istarget powerpc-*-eabi] || [istarget powerpc*-*-eabispe] || [istarget *-*-darwin*]} {
> > +	    expr 0
> 
> If ever HTM is enabled on Darwin, the testcases should Just Work as far
> as I see.

Heh, just cut/pasted the dg-do-skip from another test case, but yes,
we can remove it.  Thanks.

It's too bad we can't have a "dg-do run" and a "dg-do compile", one
used when we're on HTM hardware and the other when we're not, so we
can at least compile the test case.

Peter


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH, rs6000, testsuite] Fix PR target/64579, __TM_end __builtin_tend failed to return transactional state
  2015-03-20 22:42   ` Peter Bergner
@ 2015-04-21 20:56     ` Peter Bergner
  2015-04-22  2:18       ` Segher Boessenkool
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Bergner @ 2015-04-21 20:56 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: GCC Patches, David Edelsohn, Michael Meissner, Bill Schmidt

On Fri, 2015-03-20 at 17:41 -0500, Peter Bergner wrote:
> On Fri, 2015-03-20 at 15:52 -0500, Segher Boessenkool wrote:
> > Maybe it would be nicer if the builtin-expansion code handled the copy
> > from cc, instead of stacking on RTL expanders.
> 
> That would allow getting rid of the expanders completely, which
> would be nice.  I'd have to somehow add some type of RS6000_BTC_*
> flag onto the builtin though, so I can tell during builtin expansion
> whether I need to emit the cr copy code or not.

Ok, the patch below implements your suggestion.


> > Expanders have no constraints (you can leave out the field completely).
> > Doesn't gen* warn on non-empty constraints?
> 
> Correct, and David mentioned this when I first submitted the original
> HTM patch, but I replied they were added to allow better error
> messages when people used out of range integers for builtin args:

This is a moot point now that the expanders are gone.


> > > --- gcc/testsuite/gcc.target/powerpc/htm-1.c	(revision 0)
> > > +++ gcc/testsuite/gcc.target/powerpc/htm-1.c	(working copy)
> > > @@ -0,0 +1,53 @@
> > > +/* { dg-do run { target { powerpc*-*-* && htm_hw } } } */
> > > +/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */
> > 
> > htm_hw already disallows Darwin?  [ And {"*"} {""} is default. ]

Fixed.


This patch also fixes some issues I hit with the tabortdc[i] and
htm_m[ft]spr_<mode> patterns when used with -m32 -mpowerpc64.
This passed bootstrap and regtesting with no regressions, so
how does this look for stage1?

I'd also like to backport this to the open release branches so
everything matches (obviously waiting until after 5.1).
Is that ok once I've verified bootstrap and regtesting on
each release branch?

Peter


gcc/

	PR target/64579
	* config/rs6000/htm.md: Remove all define_expands.
	(tabort_internal, tabortdc_internal, tabortdci_internal,
	tabortwc_internal, tabortwci_internal, tbegin_internal,
	tcheck_internal, tend_internal, trechkpt_internal,
	treclaim_internal, tsr_internal): Rename define_insns from this...
	(tabort, tabortdc, tabortdci, tabortwc, tabortwci, tbegin,
	tcheck, tend, trechkpt, treclaim, tsr): ...to this.
	(tabort): Use gpc_reg_operand.
	(tabortdc, tabortdci): Match DImode registers.
	Add TARGET_POWERPC64 constraint.
	(tcheck_internal): Remove operand.
	(htm_mfspr_<mode>, htm_mtspr_<mode>): Use GPR mode macro.
	* config/rs6000/htmxlintrin.h (__TM_end): Use _HTM_TRANSACTIONAL as
	expected value.
	* config/rs6000/rs6000-builtin.def (BU_HTM_SPR0): Remove.
	(BU_HTM_SPR1): Rename to BU_HTM_V1.  Remove use of RS6000_BTC_SPR.
	(tabort, tabortdc, tabortdci, tabortwc, tabortwci, tbegin,
	tcheck, tend, tendall, trechkpt, treclaim, tresume, tsuspend,
	tsr, ttest): Pass in the RS6000_BTC_CR attribute.
	(get_tfhar, set_tfhar, get_tfiar, set_tfiar, get_texasr, set_texasr,
	get_texasru, set_texasru): Pass in the RS6000_BTC_SPR attribute.
	(tcheck): Remove builtin argument.
	(ttest): Update pattern name.
	* config/rs6000/rs6000.c (rs6000_htm_spr_icode): Use TARGET_POWERPC64
	not TARGET_64BIT.
	(htm_expand_builtin): Fix usage of expandedp.  Disallow usage of the
	tabortdc and tabortdci builtins when not in 64-bit mode.
	Modify code to handle the loss of the HTM define_expands.
	Emit code to copy the CR register to TARGET.
	(htm_init_builtins): Modify code to handle the loss of the HTM
	define_expands.
	* config/rs6000/rs6000.h (RS6000_BTC_32BIT): Delete.
	(RS6000_BTC_64BIT): Likewise.
	(RS6000_BTC_CR): New macro.
	* doc/extend.texi: Update documentation for htm builtins.

gcc/testsuite/

	PR target/64579
	* gcc.target/powerpc/htm-1.c: New test.
	* gcc.target/powerpc/htm-builtin-1.c (__builtin_tabortdc): Only test
	on 64-bit compiles.
	(__builtin_tabortdci): Likewise.
	(__builtin_tcheck): Remove operand.
	* lib/target-supports.exp (check_htm_hw_available): New function.

Index: gcc/config/rs6000/htm.md
===================================================================
--- gcc/config/rs6000/htm.md	(revision 222127)
+++ gcc/config/rs6000/htm.md	(working copy)
@@ -47,108 +47,38 @@ (define_c_enum "unspecv"
   ])
 
 
-(define_expand "tabort"
-  [(set (match_dup 2)
-	(unspec_volatile:CC [(match_operand:SI 1 "int_reg_operand" "")]
-			    UNSPECV_HTM_TABORT))
-   (set (match_dup 3)
-	(eq:SI (match_dup 2)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 3)
-		(const_int 1)))]
-  "TARGET_HTM"
-{
-  operands[2] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[3] = gen_reg_rtx (SImode);
-})
-
-(define_insn "*tabort_internal"
+(define_insn "tabort"
   [(set (match_operand:CC 1 "cc_reg_operand" "=x")
-	(unspec_volatile:CC [(match_operand:SI 0 "int_reg_operand" "r")]
+	(unspec_volatile:CC [(match_operand:SI 0 "gpc_reg_operand" "r")]
 			    UNSPECV_HTM_TABORT))]
   "TARGET_HTM"
   "tabort. %0"
   [(set_attr "type" "htm")
    (set_attr "length" "4")])
 
-(define_expand "tabortdc"
-  [(set (match_dup 4)
-	(unspec_volatile:CC [(match_operand 1 "u5bit_cint_operand" "n")
-			     (match_operand:SI 2 "gpc_reg_operand" "r")
-			     (match_operand:SI 3 "gpc_reg_operand" "r")]
-			    UNSPECV_HTM_TABORTDC))
-   (set (match_dup 5)
-	(eq:SI (match_dup 4)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 5)
-		(const_int 1)))]
-  "TARGET_HTM"
-{
-  operands[4] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[5] = gen_reg_rtx (SImode);
-})
-
-(define_insn "*tabortdc_internal"
+(define_insn "tabortdc"
   [(set (match_operand:CC 3 "cc_reg_operand" "=x")
 	(unspec_volatile:CC [(match_operand 0 "u5bit_cint_operand" "n")
-			     (match_operand:SI 1 "gpc_reg_operand" "r")
-			     (match_operand:SI 2 "gpc_reg_operand" "r")]
+			     (match_operand:DI 1 "gpc_reg_operand" "r")
+			     (match_operand:DI 2 "gpc_reg_operand" "r")]
 			    UNSPECV_HTM_TABORTDC))]
-  "TARGET_HTM"
+  "TARGET_POWERPC64 && TARGET_HTM"
   "tabortdc. %0,%1,%2"
   [(set_attr "type" "htm")
    (set_attr "length" "4")])
 
-(define_expand "tabortdci"
-  [(set (match_dup 4)
-	(unspec_volatile:CC [(match_operand 1 "u5bit_cint_operand" "n")
-			     (match_operand:SI 2 "gpc_reg_operand" "r")
-			     (match_operand 3 "s5bit_cint_operand" "n")]
-			    UNSPECV_HTM_TABORTDCI))
-   (set (match_dup 5)
-	(eq:SI (match_dup 4)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 5)
-		(const_int 1)))]
-  "TARGET_HTM"
-{
-  operands[4] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[5] = gen_reg_rtx (SImode);
-})
-
-(define_insn "*tabortdci_internal"
+(define_insn "tabortdci"
   [(set (match_operand:CC 3 "cc_reg_operand" "=x")
 	(unspec_volatile:CC [(match_operand 0 "u5bit_cint_operand" "n")
-			     (match_operand:SI 1 "gpc_reg_operand" "r")
+			     (match_operand:DI 1 "gpc_reg_operand" "r")
 			     (match_operand 2 "s5bit_cint_operand" "n")]
 			    UNSPECV_HTM_TABORTDCI))]
-  "TARGET_HTM"
+  "TARGET_POWERPC64 && TARGET_HTM"
   "tabortdci. %0,%1,%2"
   [(set_attr "type" "htm")
    (set_attr "length" "4")])
 
-(define_expand "tabortwc"
-  [(set (match_dup 4)
-	(unspec_volatile:CC [(match_operand 1 "u5bit_cint_operand" "n")
-			     (match_operand:SI 2 "gpc_reg_operand" "r")
-			     (match_operand:SI 3 "gpc_reg_operand" "r")]
-			    UNSPECV_HTM_TABORTWC))
-   (set (match_dup 5)
-	(eq:SI (match_dup 4)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 5)
-		(const_int 1)))]
-  "TARGET_HTM"
-{
-  operands[4] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[5] = gen_reg_rtx (SImode);
-})
-
-(define_insn "*tabortwc_internal"
+(define_insn "tabortwc"
   [(set (match_operand:CC 3 "cc_reg_operand" "=x")
 	(unspec_volatile:CC [(match_operand 0 "u5bit_cint_operand" "n")
 			     (match_operand:SI 1 "gpc_reg_operand" "r")
@@ -159,43 +89,7 @@ (define_insn "*tabortwc_internal"
   [(set_attr "type" "htm")
    (set_attr "length" "4")])
 
-(define_expand "tabortwci"
-  [(set (match_dup 4)
-	(unspec_volatile:CC [(match_operand 1 "u5bit_cint_operand" "n")
-			     (match_operand:SI 2 "gpc_reg_operand" "r")
-			     (match_operand 3 "s5bit_cint_operand" "n")]
-			    UNSPECV_HTM_TABORTWCI))
-   (set (match_dup 5)
-	(eq:SI (match_dup 4)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 5)
-		(const_int 1)))]
-  "TARGET_HTM"
-{
-  operands[4] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[5] = gen_reg_rtx (SImode);
-})
-
-(define_expand "ttest"
-  [(set (match_dup 1)
-	(unspec_volatile:CC [(const_int 0)
-			     (reg:SI 0)
-			     (const_int 0)]
-			    UNSPECV_HTM_TABORTWCI))
-   (set (subreg:CC (match_dup 2) 0) (match_dup 1))
-   (set (match_dup 3) (lshiftrt:SI (match_dup 2) (const_int 28)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(and:SI (match_dup 3)
-		(const_int 15)))]
-  "TARGET_HTM"
-{
-  operands[1] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[2] = gen_reg_rtx (SImode);
-  operands[3] = gen_reg_rtx (SImode);
-})
-
-(define_insn "*tabortwci_internal"
+(define_insn "tabortwci"
   [(set (match_operand:CC 3 "cc_reg_operand" "=x")
 	(unspec_volatile:CC [(match_operand 0 "u5bit_cint_operand" "n")
 			     (match_operand:SI 1 "gpc_reg_operand" "r")
@@ -206,23 +100,7 @@ (define_insn "*tabortwci_internal"
   [(set_attr "type" "htm")
    (set_attr "length" "4")])
 
-(define_expand "tbegin"
-  [(set (match_dup 2)
-	(unspec_volatile:CC [(match_operand 1 "const_0_to_1_operand" "n")]
-			    UNSPECV_HTM_TBEGIN))
-   (set (match_dup 3)
-	(eq:SI (match_dup 2)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 3)
-		(const_int 1)))]
-  "TARGET_HTM"
-{
-  operands[2] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[3] = gen_reg_rtx (SImode);
-})
-
-(define_insn "*tbegin_internal"
+(define_insn "tbegin"
   [(set (match_operand:CC 1 "cc_reg_operand" "=x")
 	(unspec_volatile:CC [(match_operand 0 "const_0_to_1_operand" "n")]
 			    UNSPECV_HTM_TBEGIN))]
@@ -231,48 +109,16 @@ (define_insn "*tbegin_internal"
   [(set_attr "type" "htm")
    (set_attr "length" "4")])
 
-(define_expand "tcheck"
-  [(set (match_dup 2)
-	(unspec_volatile:CC [(match_operand 1 "u3bit_cint_operand" "n")]
-			    UNSPECV_HTM_TCHECK))
-   (set (match_dup 3)
-	(eq:SI (match_dup 2)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 3)
-		(const_int 1)))]
-  "TARGET_HTM"
-{
-  operands[2] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[3] = gen_reg_rtx (SImode);
-})
-
-(define_insn "*tcheck_internal"
-  [(set (match_operand:CC 1 "cc_reg_operand" "=x")
-	(unspec_volatile:CC [(match_operand 0 "u3bit_cint_operand" "n")]
+(define_insn "tcheck"
+  [(set (match_operand:CC 0 "cc_reg_operand" "=y")
+	(unspec_volatile:CC [(const_int 0)]
 			    UNSPECV_HTM_TCHECK))]
   "TARGET_HTM"
   "tcheck %0"
   [(set_attr "type" "htm")
    (set_attr "length" "4")])
 
-(define_expand "tend"
-  [(set (match_dup 2)
-	(unspec_volatile:CC [(match_operand 1 "const_0_to_1_operand" "n")]
-			    UNSPECV_HTM_TEND))
-   (set (match_dup 3)
-	(eq:SI (match_dup 2)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 3)
-		(const_int 1)))]
-  "TARGET_HTM"
-{
-  operands[2] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[3] = gen_reg_rtx (SImode);
-})
-
-(define_insn "*tend_internal"
+(define_insn "tend"
   [(set (match_operand:CC 1 "cc_reg_operand" "=x")
 	(unspec_volatile:CC [(match_operand 0 "const_0_to_1_operand" "n")]
 			    UNSPECV_HTM_TEND))]
@@ -281,23 +127,7 @@ (define_insn "*tend_internal"
   [(set_attr "type" "htm")
    (set_attr "length" "4")])
 
-(define_expand "trechkpt"
-  [(set (match_dup 1)
-	(unspec_volatile:CC [(const_int 0)]
-			    UNSPECV_HTM_TRECHKPT))
-   (set (match_dup 2)
-	(eq:SI (match_dup 1)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 2)
-		(const_int 1)))]
-  "TARGET_HTM"
-{
-  operands[1] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[2] = gen_reg_rtx (SImode);
-})
-
-(define_insn "*trechkpt_internal"
+(define_insn "trechkpt"
   [(set (match_operand:CC 0 "cc_reg_operand" "=x")
 	(unspec_volatile:CC [(const_int 0)]
 			    UNSPECV_HTM_TRECHKPT))]
@@ -306,23 +136,7 @@ (define_insn "*trechkpt_internal"
   [(set_attr "type" "htm")
    (set_attr "length" "4")])
 
-(define_expand "treclaim"
-  [(set (match_dup 2)
-	(unspec_volatile:CC [(match_operand:SI 1 "gpc_reg_operand" "r")]
-			    UNSPECV_HTM_TRECLAIM))
-   (set (match_dup 3)
-	(eq:SI (match_dup 2)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 3)
-		(const_int 1)))]
-  "TARGET_HTM"
-{
-  operands[2] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[3] = gen_reg_rtx (SImode);
-})
-
-(define_insn "*treclaim_internal"
+(define_insn "treclaim"
   [(set (match_operand:CC 1 "cc_reg_operand" "=x")
 	(unspec_volatile:CC [(match_operand:SI 0 "gpc_reg_operand" "r")]
 			    UNSPECV_HTM_TRECLAIM))]
@@ -331,23 +145,7 @@ (define_insn "*treclaim_internal"
   [(set_attr "type" "htm")
    (set_attr "length" "4")])
 
-(define_expand "tsr"
-  [(set (match_dup 2)
-	(unspec_volatile:CC [(match_operand 1 "const_0_to_1_operand" "n")]
-			    UNSPECV_HTM_TSR))
-   (set (match_dup 3)
-	(eq:SI (match_dup 2)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 3)
-		(const_int 1)))]
-  "TARGET_HTM"
-{
-  operands[2] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[3] = gen_reg_rtx (SImode);
-})
-
-(define_insn "*tsr_internal"
+(define_insn "tsr"
   [(set (match_operand:CC 1 "cc_reg_operand" "=x")
 	(unspec_volatile:CC [(match_operand 0 "const_0_to_1_operand" "n")]
 			    UNSPECV_HTM_TSR))]
@@ -357,20 +155,20 @@ (define_insn "*tsr_internal"
    (set_attr "length" "4")])
 
 (define_insn "htm_mfspr_<mode>"
-  [(set (match_operand:P 0 "gpc_reg_operand" "=r")
-        (unspec_volatile:P [(match_operand 1 "u10bit_cint_operand" "n")
-			    (match_operand:P 2 "htm_spr_reg_operand" "")]
-			   UNSPECV_HTM_MFSPR))]
+  [(set (match_operand:GPR 0 "gpc_reg_operand" "=r")
+        (unspec_volatile:GPR [(match_operand 1 "u10bit_cint_operand" "n")
+			      (match_operand:GPR 2 "htm_spr_reg_operand" "")]
+			     UNSPECV_HTM_MFSPR))]
   "TARGET_HTM"
   "mfspr %0,%1";
   [(set_attr "type" "htm")
    (set_attr "length" "4")])
 
 (define_insn "htm_mtspr_<mode>"
-  [(set (match_operand:P 2 "htm_spr_reg_operand" "")
-        (unspec_volatile:P [(match_operand:P 0 "gpc_reg_operand" "r")
-			    (match_operand 1 "u10bit_cint_operand" "n")]
-                           UNSPECV_HTM_MTSPR))]
+  [(set (match_operand:GPR 2 "htm_spr_reg_operand" "")
+        (unspec_volatile:GPR [(match_operand:GPR 0 "gpc_reg_operand" "r")
+			      (match_operand 1 "u10bit_cint_operand" "n")]
+			     UNSPECV_HTM_MTSPR))]
   "TARGET_HTM"
   "mtspr %1,%0";
   [(set_attr "type" "htm")
Index: gcc/config/rs6000/htmxlintrin.h
===================================================================
--- gcc/config/rs6000/htmxlintrin.h	(revision 222127)
+++ gcc/config/rs6000/htmxlintrin.h	(working copy)
@@ -81,7 +81,8 @@ extern __inline long
 __attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
 __TM_end (void)
 {
-  if (__builtin_expect (__builtin_tend (0), 1))
+  unsigned char status = _HTM_STATE (__builtin_tend (0));
+  if (__builtin_expect (status, _HTM_TRANSACTIONAL))
     return 1;
   return 0;
 }
Index: gcc/config/rs6000/rs6000-builtin.def
===================================================================
--- gcc/config/rs6000/rs6000-builtin.def	(revision 222127)
+++ gcc/config/rs6000/rs6000-builtin.def	(working copy)
@@ -456,21 +456,12 @@
 		     | RS6000_BTC_TERNARY),				\
 		    CODE_FOR_ ## ICODE)			/* ICODE */
 
-#define BU_HTM_SPR0(ENUM, NAME, ATTR, ICODE)				\
-  RS6000_BUILTIN_H (HTM_BUILTIN_ ## ENUM,		/* ENUM */	\
-		    "__builtin_" NAME,			/* NAME */	\
-		    RS6000_BTM_HTM,			/* MASK */	\
-		    (RS6000_BTC_ ## ATTR		/* ATTR */	\
-		     | RS6000_BTC_SPR),					\
-		    CODE_FOR_ ## ICODE)			/* ICODE */
-
-#define BU_HTM_SPR1(ENUM, NAME, ATTR, ICODE)				\
+#define BU_HTM_V1(ENUM, NAME, ATTR, ICODE)				\
   RS6000_BUILTIN_H (HTM_BUILTIN_ ## ENUM,		/* ENUM */	\
 		    "__builtin_" NAME,			/* NAME */	\
 		    RS6000_BTM_HTM,			/* MASK */	\
 		    (RS6000_BTC_ ## ATTR		/* ATTR */	\
 		     | RS6000_BTC_UNARY					\
-		     | RS6000_BTC_SPR					\
 		     | RS6000_BTC_VOID),				\
 		    CODE_FOR_ ## ICODE)			/* ICODE */
 
@@ -1633,30 +1624,30 @@ BU_CRYPTO_OVERLOAD_3 (VSHASIGMA, "vshasi
 
 \f
 /* HTM functions.  */
-BU_HTM_1  (TABORT,	"tabort",	MISC,	tabort)
-BU_HTM_3  (TABORTDC,	"tabortdc",	MISC,	tabortdc)
-BU_HTM_3  (TABORTDCI,	"tabortdci",	MISC,	tabortdci)
-BU_HTM_3  (TABORTWC,	"tabortwc",	MISC,	tabortwc)
-BU_HTM_3  (TABORTWCI,	"tabortwci",	MISC,	tabortwci)
-BU_HTM_1  (TBEGIN,	"tbegin",	MISC,	tbegin)
-BU_HTM_1  (TCHECK,	"tcheck",	MISC,	tcheck)
-BU_HTM_1  (TEND,	"tend",		MISC,	tend)
-BU_HTM_0  (TENDALL,	"tendall",	MISC,	tend)
-BU_HTM_0  (TRECHKPT,	"trechkpt",	MISC,	trechkpt)
-BU_HTM_1  (TRECLAIM,	"treclaim",	MISC,	treclaim)
-BU_HTM_0  (TRESUME,	"tresume",	MISC,	tsr)
-BU_HTM_0  (TSUSPEND,	"tsuspend",	MISC,	tsr)
-BU_HTM_1  (TSR,		"tsr",		MISC,	tsr)
-BU_HTM_0  (TTEST,	"ttest",	MISC,	ttest)
-
-BU_HTM_SPR0 (GET_TFHAR,		"get_tfhar",	MISC,	nothing)
-BU_HTM_SPR1 (SET_TFHAR,		"set_tfhar",	MISC,	nothing)
-BU_HTM_SPR0 (GET_TFIAR,		"get_tfiar",	MISC,	nothing)
-BU_HTM_SPR1 (SET_TFIAR,		"set_tfiar",	MISC,	nothing)
-BU_HTM_SPR0 (GET_TEXASR,	"get_texasr",	MISC,	nothing)
-BU_HTM_SPR1 (SET_TEXASR,	"set_texasr",	MISC,	nothing)
-BU_HTM_SPR0 (GET_TEXASRU,	"get_texasru",	MISC,	nothing)
-BU_HTM_SPR1 (SET_TEXASRU,	"set_texasru",	MISC,	nothing)
+BU_HTM_1  (TABORT,	"tabort",	CR,	tabort)
+BU_HTM_3  (TABORTDC,	"tabortdc",	CR,	tabortdc)
+BU_HTM_3  (TABORTDCI,	"tabortdci",	CR,	tabortdci)
+BU_HTM_3  (TABORTWC,	"tabortwc",	CR,	tabortwc)
+BU_HTM_3  (TABORTWCI,	"tabortwci",	CR,	tabortwci)
+BU_HTM_1  (TBEGIN,	"tbegin",	CR,	tbegin)
+BU_HTM_0  (TCHECK,	"tcheck",	CR,	tcheck)
+BU_HTM_1  (TEND,	"tend",		CR,	tend)
+BU_HTM_0  (TENDALL,	"tendall",	CR,	tend)
+BU_HTM_0  (TRECHKPT,	"trechkpt",	CR,	trechkpt)
+BU_HTM_1  (TRECLAIM,	"treclaim",	CR,	treclaim)
+BU_HTM_0  (TRESUME,	"tresume",	CR,	tsr)
+BU_HTM_0  (TSUSPEND,	"tsuspend",	CR,	tsr)
+BU_HTM_1  (TSR,		"tsr",		CR,	tsr)
+BU_HTM_0  (TTEST,	"ttest",	CR,	tabortwci)
+
+BU_HTM_0  (GET_TFHAR,	"get_tfhar",	SPR,	nothing)
+BU_HTM_V1 (SET_TFHAR,	"set_tfhar",	SPR,	nothing)
+BU_HTM_0  (GET_TFIAR,	"get_tfiar",	SPR,	nothing)
+BU_HTM_V1 (SET_TFIAR,	"set_tfiar",	SPR,	nothing)
+BU_HTM_0  (GET_TEXASR,	"get_texasr",	SPR,	nothing)
+BU_HTM_V1 (SET_TEXASR,	"set_texasr",	SPR,	nothing)
+BU_HTM_0  (GET_TEXASRU,	"get_texasru",	SPR,	nothing)
+BU_HTM_V1 (SET_TEXASRU,	"set_texasru",	SPR,	nothing)
 
 \f
 /* 3 argument paired floating point builtins.  */
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 222127)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -12655,9 +12655,9 @@ static inline enum insn_code
 rs6000_htm_spr_icode (bool nonvoid)
 {
   if (nonvoid)
-    return (TARGET_64BIT) ? CODE_FOR_htm_mfspr_di : CODE_FOR_htm_mfspr_si;
+    return (TARGET_POWERPC64) ? CODE_FOR_htm_mfspr_di : CODE_FOR_htm_mfspr_si;
   else
-    return (TARGET_64BIT) ? CODE_FOR_htm_mtspr_di : CODE_FOR_htm_mtspr_si;
+    return (TARGET_POWERPC64) ? CODE_FOR_htm_mtspr_di : CODE_FOR_htm_mtspr_si;
 }
 
 /* Expand the HTM builtin in EXP and store the result in TARGET.
@@ -12671,7 +12671,17 @@ htm_expand_builtin (tree exp, rtx target
   const struct builtin_description *d;
   size_t i;
 
-  *expandedp = false;
+  *expandedp = true;
+
+  if (!TARGET_POWERPC64
+      && (fcode == HTM_BUILTIN_TABORTDC
+	  || fcode == HTM_BUILTIN_TABORTDCI))
+    {
+      size_t uns_fcode = (size_t)fcode;
+      const char *name = rs6000_builtin_info[uns_fcode].name;
+      error ("builtin %s is only valid in 64-bit mode", name);
+      return const0_rtx;
+    }
 
   /* Expand the HTM builtins.  */
   d = bdesc_htm;
@@ -12684,26 +12694,29 @@ htm_expand_builtin (tree exp, rtx target
 	call_expr_arg_iterator iter;
 	unsigned attr = rs6000_builtin_info[fcode].attr;
 	enum insn_code icode = d->icode;
+	const struct insn_operand_data *insn_op;
+	bool uses_spr = (attr & RS6000_BTC_SPR);
+	rtx cr = NULL_RTX;
 
-	if (attr & RS6000_BTC_SPR)
+	if (uses_spr)
 	  icode = rs6000_htm_spr_icode (nonvoid);
+	insn_op = &insn_data[icode].operand[0];
 
 	if (nonvoid)
 	  {
-	    machine_mode tmode = insn_data[icode].operand[0].mode;
+	    machine_mode tmode = (uses_spr) ? insn_op->mode : SImode;
 	    if (!target
 		|| GET_MODE (target) != tmode
-		|| !(*insn_data[icode].operand[0].predicate) (target, tmode))
+		|| (uses_spr && !(*insn_op->predicate) (target, tmode)))
 	      target = gen_reg_rtx (tmode);
-	    op[nopnds++] = target;
+	    if (uses_spr)
+	      op[nopnds++] = target;
 	  }
 
 	FOR_EACH_CALL_EXPR_ARG (arg, iter, exp)
 	{
-	  const struct insn_operand_data *insn_op;
-
 	  if (arg == error_mark_node || nopnds >= MAX_HTM_OPERANDS)
-	    return NULL_RTX;
+	    return const0_rtx;
 
 	  insn_op = &insn_data[icode].operand[nopnds];
 
@@ -12744,16 +12757,31 @@ htm_expand_builtin (tree exp, rtx target
 	    attr |= RS6000_BTC_UNARY;
 #endif
 	    break;
+	  case HTM_BUILTIN_TTEST: /* Alias for: tabortwci. 0,r0,0  */
+	    op[nopnds++] = GEN_INT (0);
+	    op[nopnds++] = gen_rtx_REG (SImode, 0);
+	    op[nopnds++] = GEN_INT (0);
+#ifdef ENABLE_CHECKING
+	    attr |= RS6000_BTC_TERNARY;
+#endif
+	    break;
 	  default:
 	    break;
 	  }
 
 	/* If this builtin accesses SPRs, then pass in the appropriate
 	   SPR number and SPR regno as the last two operands.  */
-	if (attr & RS6000_BTC_SPR)
+	if (uses_spr)
 	  {
-	    op[nopnds++] = gen_rtx_CONST_INT (Pmode, htm_spr_num (fcode));
-	    op[nopnds++] = gen_rtx_REG (Pmode, htm_spr_regno (fcode));
+	    machine_mode mode = (TARGET_POWERPC64) ? DImode : SImode;
+	    op[nopnds++] = gen_rtx_CONST_INT (mode, htm_spr_num (fcode));
+	    op[nopnds++] = gen_rtx_REG (mode, htm_spr_regno (fcode));
+	  }
+	/* If this builtin accesses a CR, then pass in a scratch
+	   CR as the last operand.  */
+	else if (attr & RS6000_BTC_CR)
+	  { cr = gen_reg_rtx (CCmode);
+	    op[nopnds++] = cr;
 	  }
 
 #ifdef ENABLE_CHECKING
@@ -12766,7 +12794,7 @@ htm_expand_builtin (tree exp, rtx target
 	  expected_nopnds = 3;
 	if (!(attr & RS6000_BTC_VOID))
 	  expected_nopnds += 1;
-	if (attr & RS6000_BTC_SPR)
+	if (uses_spr)
 	  expected_nopnds += 2;
 
 	gcc_assert (nopnds == expected_nopnds && nopnds <= MAX_HTM_OPERANDS);
@@ -12793,12 +12821,41 @@ htm_expand_builtin (tree exp, rtx target
 	  return NULL_RTX;
 	emit_insn (pat);
 
-	*expandedp = true;
+	if (attr & RS6000_BTC_CR)
+	  {
+	    if (fcode == HTM_BUILTIN_TBEGIN)
+	      {
+		/* Emit code to set TARGET to true or false depending on
+		   whether the tbegin. instruction successfully or failed
+		   to start a transaction.  We do this by placing the 1's
+		   complement of CR's EQ bit into TARGET.  */
+		rtx scratch = gen_reg_rtx (SImode);
+		emit_insn (gen_rtx_SET (VOIDmode, scratch,
+					gen_rtx_EQ (SImode, cr,
+						     const0_rtx)));
+		emit_insn (gen_rtx_SET (VOIDmode, target,
+					gen_rtx_XOR (SImode, scratch,
+						     GEN_INT (1))));
+	      }
+	    else
+	      {
+		/* Emit code to copy the 4-bit condition register field
+		   CR into the least significant end of register TARGET.  */
+		rtx scratch1 = gen_reg_rtx (SImode);
+		rtx scratch2 = gen_reg_rtx (SImode);
+		rtx subreg = simplify_gen_subreg (CCmode, scratch1, SImode, 0);
+		emit_insn (gen_movcc (subreg, cr));
+		emit_insn (gen_lshrsi3 (scratch2, scratch1, GEN_INT (28)));
+		emit_insn (gen_andsi3 (target, scratch2, GEN_INT (0xf)));
+	      }
+	  }
+
 	if (nonvoid)
 	  return target;
 	return const0_rtx;
       }
 
+  *expandedp = false;
   return NULL_RTX;
 }
 
@@ -15287,8 +15344,31 @@ htm_init_builtins (void)
       bool void_func = (attr & RS6000_BTC_VOID);
       int attr_args = (attr & RS6000_BTC_TYPE_MASK);
       int nopnds = 0;
-      tree argtype = (attr & RS6000_BTC_SPR) ? long_unsigned_type_node
-					     : unsigned_type_node;
+      tree gpr_type_node;
+      tree rettype;
+      tree argtype;
+
+      if (TARGET_32BIT && TARGET_POWERPC64)
+	gpr_type_node = long_long_unsigned_type_node;
+      else
+	gpr_type_node = long_unsigned_type_node;
+
+      if (attr & RS6000_BTC_SPR)
+	{
+	  rettype = gpr_type_node;
+	  argtype = gpr_type_node;
+	}
+      else if (d->code == HTM_BUILTIN_TABORTDC
+	       || d->code == HTM_BUILTIN_TABORTDCI)
+	{
+	  rettype = unsigned_type_node;
+	  argtype = gpr_type_node;
+	}
+      else
+	{
+	  rettype = unsigned_type_node;
+	  argtype = unsigned_type_node;
+	}
 
       if ((mask & builtin_mask) != mask)
 	{
@@ -15305,7 +15385,7 @@ htm_init_builtins (void)
 	  continue;
 	}
 
-      op[nopnds++] = (void_func) ? void_type_node : argtype;
+      op[nopnds++] = (void_func) ? void_type_node : rettype;
 
       if (attr_args == RS6000_BTC_UNARY)
 	op[nopnds++] = argtype;
Index: gcc/config/rs6000/rs6000.h
===================================================================
--- gcc/config/rs6000/rs6000.h	(revision 222127)
+++ gcc/config/rs6000/rs6000.h	(working copy)
@@ -2573,9 +2573,8 @@ extern int frame_pointer_needed;
 /* Miscellaneous information.  */
 #define RS6000_BTC_SPR		0x01000000	/* function references SPRs.  */
 #define RS6000_BTC_VOID		0x02000000	/* function has no return value.  */
-#define RS6000_BTC_OVERLOADED	0x04000000	/* function is overloaded.  */
-#define RS6000_BTC_32BIT	0x08000000	/* function references SPRs.  */
-#define RS6000_BTC_64BIT	0x10000000	/* function references SPRs.  */
+#define RS6000_BTC_CR		0x04000000	/* function references a CR.  */
+#define RS6000_BTC_OVERLOADED	0x08000000	/* function is overloaded.  */
 #define RS6000_BTC_MISC_MASK	0x1f000000	/* Mask of the misc info.  */
 
 /* Convenience macros to document the instruction type.  */
Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 222127)
+++ gcc/doc/extend.texi	(working copy)
@@ -15121,10 +15121,15 @@ The following low level built-in functio
 @option{-mhtm} or @option{-mcpu=CPU} where CPU is `power8' or later.
 They all generate the machine instruction that is part of the name.
 
-The HTM built-ins return true or false depending on their success and
-their arguments match exactly the type and order of the associated
-hardware instruction's operands.  Refer to the ISA manual for a
-description of each instruction's operands.
+The HTM builtins (with the exception of @code{__builtin_tbegin}) return
+the full 4-bit condition register value set by their associated hardware
+instruction.  The header file @code{htmintrin.h} defines some macros that can
+be used to decipher the return value.  The @code{__builtin_tbegin} builtin
+returns a simple true or false value depending on whether a transaction was
+successfully started or not.  The arguments of the builtins match exactly the
+type and order of the associated hardware instruction's operands, except for
+the @code{__builtin_tcheck} builtin, which does not take any input arguments.
+Refer to the ISA manual for a description of each instruction's operands.
 
 @smallexample
 unsigned int __builtin_tbegin (unsigned int)
@@ -15136,7 +15141,7 @@ unsigned int __builtin_tabortdci (unsign
 unsigned int __builtin_tabortwc (unsigned int, unsigned int, unsigned int)
 unsigned int __builtin_tabortwci (unsigned int, unsigned int, int)
 
-unsigned int __builtin_tcheck (unsigned int)
+unsigned int __builtin_tcheck (void)
 unsigned int __builtin_treclaim (unsigned int)
 unsigned int __builtin_trechkpt (void)
 unsigned int __builtin_tsr (unsigned int)
@@ -15271,7 +15276,7 @@ TM_buff_type TM_buff;
 
 while (1)
   @{
-    if (__TM_begin (TM_buff))
+    if (__TM_begin (TM_buff) == _HTM_TBEGIN_STARTED)
       @{
         /* Transaction State Initiated.  */
         if (is_locked (lock))
Index: gcc/testsuite/gcc.target/powerpc/htm-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/htm-1.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/htm-1.c	(working copy)
@@ -0,0 +1,52 @@
+/* { dg-do run { target { powerpc*-*-* && htm_hw } } } */
+/* { dg-require-effective-target powerpc_htm_ok } */
+/* { dg-options "-mhtm" } */
+
+/* Program to test PowerPC HTM instructions.  */
+
+#include <stdlib.h>
+#include <htmintrin.h>
+
+int
+main (void)
+{
+  long i;
+  unsigned long mask = 0;
+
+repeat:
+  if (__builtin_tbegin (0))
+    {
+      mask++;
+    }
+  else
+    abort();
+
+  if (mask == 1)
+    {
+      __builtin_tsuspend ();
+
+      if (_HTM_STATE (__builtin_tcheck ()) != _HTM_SUSPENDED)
+	abort ();
+
+      __builtin_tresume ();
+
+      if (_HTM_STATE (__builtin_tcheck ()) != _HTM_TRANSACTIONAL)
+	abort ();
+    }
+  else
+    mask++;
+
+  if (_HTM_STATE (__builtin_tendall ()) != _HTM_TRANSACTIONAL)
+    abort ();
+
+  if (mask == 1)
+    goto repeat;
+
+  if (_HTM_STATE (__builtin_tendall ()) != _HTM_NONTRANSACTIONAL)
+    abort ();
+
+  if (mask != 3)
+    abort ();
+
+  return 0;
+}
Index: gcc/testsuite/gcc.target/powerpc/htm-builtin-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/htm-builtin-1.c	(revision 222127)
+++ gcc/testsuite/gcc.target/powerpc/htm-builtin-1.c	(working copy)
@@ -6,8 +6,8 @@
 /* { dg-final { scan-assembler-times "tbegin\\." 1 } } */
 /* { dg-final { scan-assembler-times "tend\\." 2 } } */
 /* { dg-final { scan-assembler-times "tabort\\." 2 } } */
-/* { dg-final { scan-assembler-times "tabortdc\\." 1 } } */
-/* { dg-final { scan-assembler-times "tabortdci\\." 1 } } */
+/* { dg-final { scan-assembler-times "tabortdc\\." 1 { target lp64 } } } */
+/* { dg-final { scan-assembler-times "tabortdci\\." 1 { target lp64 } } } */
 /* { dg-final { scan-assembler-times "tabortwc\\." 1 } } */
 /* { dg-final { scan-assembler-times "tabortwci\\." 2 } } */
 /* { dg-final { scan-assembler-times "tcheck" 1 } } */
@@ -25,12 +25,14 @@ void use_builtins (long *p, char code, l
   p[3] = __builtin_tabort (0);
   p[4] = __builtin_tabort (code);
 
+#ifdef __powerpc64__
   p[5] = __builtin_tabortdc (0xf, a[5], b[5]);
   p[6] = __builtin_tabortdci (0xf, a[6], 13);
+#endif
   p[7] = __builtin_tabortwc (0xf, a[7], b[7]);
   p[8] = __builtin_tabortwci (0xf, a[8], 13);
 
-  p[9] = __builtin_tcheck (5);
+  p[9] = __builtin_tcheck ();
   p[10] = __builtin_trechkpt ();
   p[11] = __builtin_treclaim (0);
   p[12] = __builtin_tresume ();
Index: gcc/testsuite/lib/target-supports.exp
===================================================================
--- gcc/testsuite/lib/target-supports.exp	(revision 222127)
+++ gcc/testsuite/lib/target-supports.exp	(working copy)
@@ -3279,6 +3279,25 @@ proc check_effective_target_powerpc_htm_
     }
 }
 
+# Return 1 if the target supports executing HTM hardware instructions,
+# 0 otherwise.  Cache the result.
+
+proc check_htm_hw_available { } {
+    return [check_cached_effective_target htm_hw_available {
+	# For now, disable on Darwin
+	if { [istarget powerpc-*-eabi] || [istarget powerpc*-*-eabispe] || [istarget *-*-darwin*]} {
+	    expr 0
+	} else {
+	    check_runtime_nocache htm_hw_available {
+		int main()
+		{
+		  __builtin_ttest ();
+		  return 0;
+		}
+	    } "-mhtm"
+	}
+    }]
+}
 # Return 1 if this is a PowerPC target supporting -mcpu=cell.
 
 proc check_effective_target_powerpc_ppu_ok { } {
@@ -5278,6 +5297,7 @@ proc is-effective-target { arg } {
 	  "p8vector_hw"    { set selected [check_p8vector_hw_available] }
 	  "ppc_recip_hw"   { set selected [check_ppc_recip_hw_available] }
 	  "dfp_hw"         { set selected [check_dfp_hw_available] }
+	  "htm_hw"         { set selected [check_htm_hw_available] }
 	  "named_sections" { set selected [check_named_sections_available] }
 	  "gc_sections"    { set selected [check_gc_sections_available] }
 	  "cxa_atexit"     { set selected [check_cxa_atexit_available] }
@@ -5301,6 +5321,7 @@ proc is-effective-target-keyword { arg }
 	  "p8vector_hw"    { return 1 }
 	  "ppc_recip_hw"   { return 1 }
 	  "dfp_hw"         { return 1 }
+	  "htm_hw"         { return 1 }
 	  "named_sections" { return 1 }
 	  "gc_sections"    { return 1 }
 	  "cxa_atexit"     { return 1 }


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH, rs6000, testsuite] Fix PR target/64579, __TM_end __builtin_tend failed to return transactional state
  2015-04-21 20:56     ` Peter Bergner
@ 2015-04-22  2:18       ` Segher Boessenkool
  2015-04-22 13:43         ` Peter Bergner
  0 siblings, 1 reply; 14+ messages in thread
From: Segher Boessenkool @ 2015-04-22  2:18 UTC (permalink / raw)
  To: Peter Bergner; +Cc: GCC Patches, David Edelsohn, Michael Meissner, Bill Schmidt

On Tue, Apr 21, 2015 at 03:56:18PM -0500, Peter Bergner wrote:
> On Fri, 2015-03-20 at 17:41 -0500, Peter Bergner wrote:
> > On Fri, 2015-03-20 at 15:52 -0500, Segher Boessenkool wrote:
> > > Maybe it would be nicer if the builtin-expansion code handled the copy
> > > from cc, instead of stacking on RTL expanders.
> > 
> > That would allow getting rid of the expanders completely, which
> > would be nice.  I'd have to somehow add some type of RS6000_BTC_*
> > flag onto the builtin though, so I can tell during builtin expansion
> > whether I need to emit the cr copy code or not.
> 
> Ok, the patch below implements your suggestion.

It looks good, thanks.  Some minor comments...

> This patch also fixes some issues I hit with the tabortdc[i] and
> htm_m[ft]spr_<mode> patterns when used with -m32 -mpowerpc64.

Running the testsuite, or did you actually try to _use_ -m32 -mpowerpc64?  :-)

> +(define_insn "tabortdc"
>    [(set (match_operand:CC 3 "cc_reg_operand" "=x")
>  	(unspec_volatile:CC [(match_operand 0 "u5bit_cint_operand" "n")
> -			     (match_operand:SI 1 "gpc_reg_operand" "r")
> -			     (match_operand:SI 2 "gpc_reg_operand" "r")]
> +			     (match_operand:DI 1 "gpc_reg_operand" "r")
> +			     (match_operand:DI 2 "gpc_reg_operand" "r")]
>  			    UNSPECV_HTM_TABORTDC))]
> -  "TARGET_HTM"
> +  "TARGET_POWERPC64 && TARGET_HTM"
>    "tabortdc. %0,%1,%2"
>    [(set_attr "type" "htm")
>     (set_attr "length" "4")])

Maybe you can fold tabortdc with tabortwc now?  Use one UNSPEC name
for both, :GPR and <wd>?

> +	  case HTM_BUILTIN_TTEST: /* Alias for: tabortwci. 0,r0,0  */
> +	    op[nopnds++] = GEN_INT (0);
> +	    op[nopnds++] = gen_rtx_REG (SImode, 0);
> +	    op[nopnds++] = GEN_INT (0);

Is that really r0, isn't that (0|rA)?  [Too lazy to read the docs myself
right now, sorry.]

> +	if (attr & RS6000_BTC_CR)
> +	  {
> +	    if (fcode == HTM_BUILTIN_TBEGIN)
> +	      {
> +		/* Emit code to set TARGET to true or false depending on
> +		   whether the tbegin. instruction successfully or failed
> +		   to start a transaction.  We do this by placing the 1's
> +		   complement of CR's EQ bit into TARGET.  */
> +		rtx scratch = gen_reg_rtx (SImode);
> +		emit_insn (gen_rtx_SET (VOIDmode, scratch,
> +					gen_rtx_EQ (SImode, cr,
> +						     const0_rtx)));
> +		emit_insn (gen_rtx_SET (VOIDmode, target,
> +					gen_rtx_XOR (SImode, scratch,
> +						     GEN_INT (1))));
> +	      }
> +	    else
> +	      {
> +		/* Emit code to copy the 4-bit condition register field
> +		   CR into the least significant end of register TARGET.  */
> +		rtx scratch1 = gen_reg_rtx (SImode);
> +		rtx scratch2 = gen_reg_rtx (SImode);
> +		rtx subreg = simplify_gen_subreg (CCmode, scratch1, SImode, 0);
> +		emit_insn (gen_movcc (subreg, cr));
> +		emit_insn (gen_lshrsi3 (scratch2, scratch1, GEN_INT (28)));
> +		emit_insn (gen_andsi3 (target, scratch2, GEN_INT (0xf)));
> +	      }
> +	  }

Don't we have helper functions/expanders to do these moves?  Yuck.

> -/* { dg-final { scan-assembler-times "tabortdc\\." 1 } } */
> -/* { dg-final { scan-assembler-times "tabortdci\\." 1 } } */
> +/* { dg-final { scan-assembler-times "tabortdc\\." 1 { target lp64 } } } */
> +/* { dg-final { scan-assembler-times "tabortdci\\." 1 { target lp64 } } } */

This skips this test on -m32 -mpowerpc64, is that on purpose?


Segher

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH, rs6000, testsuite] Fix PR target/64579, __TM_end __builtin_tend failed to return transactional state
  2015-04-22  2:18       ` Segher Boessenkool
@ 2015-04-22 13:43         ` Peter Bergner
  2015-04-22 22:16           ` Segher Boessenkool
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Bergner @ 2015-04-22 13:43 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: GCC Patches, David Edelsohn, Michael Meissner, Bill Schmidt

On Tue, 2015-04-21 at 21:17 -0500, Segher Boessenkool wrote:
> On Tue, Apr 21, 2015 at 03:56:18PM -0500, Peter Bergner wrote:
> > This patch also fixes some issues I hit with the tabortdc[i] and
> > htm_m[ft]spr_<mode> patterns when used with -m32 -mpowerpc64.
> 
> Running the testsuite, or did you actually try to _use_ -m32 -mpowerpc64?  :-)

Not with the testsuite.  I had some simple unit tests that basically
just returned the CR/SPR and hit some ICEs.





> Maybe you can fold tabortdc with tabortwc now?  Use one UNSPEC name
> for both, :GPR and <wd>?

Wouldn't that change the tabortwc pattern to use DImode rather
than SImode when compiled with -m64 or -m32 -mpowerpc64?
I'm not sure we want that.



> > +	  case HTM_BUILTIN_TTEST: /* Alias for: tabortwci. 0,r0,0  */
> > +	    op[nopnds++] = GEN_INT (0);
> > +	    op[nopnds++] = gen_rtx_REG (SImode, 0);
> > +	    op[nopnds++] = GEN_INT (0);
> 
> Is that really r0, isn't that (0|rA)?  [Too lazy to read the docs myself
> right now, sorry.]

The ISA doc shows:

  tabortwci. TO,RA,SI

  a <- EXTS((RA)32:63)
  abort <- 0
  CR0 <- 0 || MSR(TS) || 0

  if a < EXTS(SI) & TO0 then abort <- 1
  if a > EXTS(SI) & TO1 then abort <- 1
  if a = EXTS(SI) & T02 then abort <- 1
  if a u< EXTS(SI) & TO3 then abort <- 1
  if a >u EXTS(SI) & TO4 then abort <- 1

  ...

Given that I'm passing in a zero TO value, the second and third
operands are don't care values, so I'm just using r0 and 0 as
random input values.  I'm only interested in extracting the
MSR's Transaction Status (TS) bits and placing them into CR0.



> > +		emit_insn (gen_movcc (subreg, cr));
> > +		emit_insn (gen_lshrsi3 (scratch2, scratch1, GEN_INT (28)));
> > +		emit_insn (gen_andsi3 (target, scratch2, GEN_INT (0xf)));
> > +	      }
> > +	  }
> 
> Don't we have helper functions/expanders to do these moves?  Yuck.

Heh, I looked.  The only helper pattern was the movcc pattern, but
that placed the CR into bits 32-35 of the register.  I needed the
shift to move it down into the low nibble and I use the "and", since
one of the move cr insns places two copies of the CR value into
bits 32-35 and 36-39.


> > -/* { dg-final { scan-assembler-times "tabortdc\\." 1 } } */
> > -/* { dg-final { scan-assembler-times "tabortdci\\." 1 } } */
> > +/* { dg-final { scan-assembler-times "tabortdc\\." 1 { target lp64 } } } */
> > +/* { dg-final { scan-assembler-times "tabortdci\\." 1 { target lp64 } } } */
> 
> This skips this test on -m32 -mpowerpc64, is that on purpose?

Ummm, not exactly.  :-)   Not that many people test that though.
I'll see if I can find a replacement for lp64 that covers that case.
If not, I'm not too torn up if we skip it for -m32 -mpowerpc64.


Peter



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH, rs6000, testsuite] Fix PR target/64579, __TM_end __builtin_tend failed to return transactional state
  2015-04-22 13:43         ` Peter Bergner
@ 2015-04-22 22:16           ` Segher Boessenkool
  2015-04-22 23:08             ` Peter Bergner
  2015-04-23 19:16             ` Peter Bergner
  0 siblings, 2 replies; 14+ messages in thread
From: Segher Boessenkool @ 2015-04-22 22:16 UTC (permalink / raw)
  To: Peter Bergner; +Cc: GCC Patches, David Edelsohn, Michael Meissner, Bill Schmidt

On Wed, Apr 22, 2015 at 08:43:10AM -0500, Peter Bergner wrote:
> > Maybe you can fold tabortdc with tabortwc now?  Use one UNSPEC name
> > for both, :GPR and <wd>?
> 
> Wouldn't that change the tabortwc pattern to use DImode rather
> than SImode when compiled with -m64 or -m32 -mpowerpc64?
> I'm not sure we want that.

The GPR mode iterator creates two patterns, one for SI and one for DI,
and the tabortwc would be the one for SI if you use <wd>.

> > > +	  case HTM_BUILTIN_TTEST: /* Alias for: tabortwci. 0,r0,0  */
> > > +	    op[nopnds++] = GEN_INT (0);
> > > +	    op[nopnds++] = gen_rtx_REG (SImode, 0);
> > > +	    op[nopnds++] = GEN_INT (0);
> > 
> > Is that really r0, isn't that (0|rA)?  [Too lazy to read the docs myself
> > right now, sorry.]
> 
> The ISA doc shows:

[snip]

Thanks for looking it up!

I'm still a bit worried about putting a reg in the RTL (while the instruction
doesn't actually use one), but perhaps it's harmless.

> > > +		emit_insn (gen_movcc (subreg, cr));
> > > +		emit_insn (gen_lshrsi3 (scratch2, scratch1, GEN_INT (28)));
> > > +		emit_insn (gen_andsi3 (target, scratch2, GEN_INT (0xf)));
> > > +	      }
> > > +	  }
> > 
> > Don't we have helper functions/expanders to do these moves?  Yuck.
> 
> Heh, I looked.  The only helper pattern was the movcc pattern, but
> that placed the CR into bits 32-35 of the register.  I needed the
> shift to move it down into the low nibble and I use the "and", since
> one of the move cr insns places two copies of the CR value into
> bits 32-35 and 36-39.

At least the VMX patterns have something like it for CR6.  Probably not
directly usable either, sigh.

> > > -/* { dg-final { scan-assembler-times "tabortdc\\." 1 } } */
> > > -/* { dg-final { scan-assembler-times "tabortdci\\." 1 } } */
> > > +/* { dg-final { scan-assembler-times "tabortdc\\." 1 { target lp64 } } } */
> > > +/* { dg-final { scan-assembler-times "tabortdci\\." 1 { target lp64 } } } */
> > 
> > This skips this test on -m32 -mpowerpc64, is that on purpose?
> 
> Ummm, not exactly.  :-)   Not that many people test that though.
> I'll see if I can find a replacement for lp64 that covers that case.

Maybe just { powerpc64 } works?

> If not, I'm not too torn up if we skip it for -m32 -mpowerpc64.

Me neither, just looked like an oversight.


Segher

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH, rs6000, testsuite] Fix PR target/64579, __TM_end __builtin_tend failed to return transactional state
  2015-04-22 22:16           ` Segher Boessenkool
@ 2015-04-22 23:08             ` Peter Bergner
  2015-04-23  1:55               ` Segher Boessenkool
  2015-04-23 19:16             ` Peter Bergner
  1 sibling, 1 reply; 14+ messages in thread
From: Peter Bergner @ 2015-04-22 23:08 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: GCC Patches, David Edelsohn, Michael Meissner, Bill Schmidt

On Wed, 2015-04-22 at 17:16 -0500, Segher Boessenkool wrote:
> On Wed, Apr 22, 2015 at 08:43:10AM -0500, Peter Bergner wrote:
> > > Maybe you can fold tabortdc with tabortwc now?  Use one UNSPEC name
> > > for both, :GPR and <wd>?
> > 
> > Wouldn't that change the tabortwc pattern to use DImode rather
> > than SImode when compiled with -m64 or -m32 -mpowerpc64?
> > I'm not sure we want that.
> 
> The GPR mode iterator creates two patterns, one for SI and one for DI,
> and the tabortwc would be the one for SI if you use <wd>.

Ah, I think I know what you mean.  Sure, I can try that,





> > > > +	  case HTM_BUILTIN_TTEST: /* Alias for: tabortwci. 0,r0,0  */
> > > > +	    op[nopnds++] = GEN_INT (0);
> > > > +	    op[nopnds++] = gen_rtx_REG (SImode, 0);
> > > > +	    op[nopnds++] = GEN_INT (0);
> > > 
> > > Is that really r0, isn't that (0|rA)?  [Too lazy to read the docs myself
> > > right now, sorry.]
> > 
> > The ISA doc shows:
> 
> [snip]
> 
> Thanks for looking it up!
> 
> I'm still a bit worried about putting a reg in the RTL (while the instruction
> doesn't actually use one), but perhaps it's harmless.

I'm not sure what you mean by the "instruction doesn't use one".
The hardware instruction does use a register for its second
operand (even though its contents are ignored due to TO == 0)
and the pattern requires us to pass in a reg rtx, so I'm not
sure what you're referring to.





> >> This skips this test on -m32 -mpowerpc64, is that on purpose?
> > 
> > Ummm, not exactly.  :-)   Not that many people test that though.
> > I'll see if I can find a replacement for lp64 that covers that case.
> 
> Maybe just { powerpc64 } works?

I'll take a look at that to see if that works, thanks.

Peter



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH, rs6000, testsuite] Fix PR target/64579, __TM_end __builtin_tend failed to return transactional state
  2015-04-22 23:08             ` Peter Bergner
@ 2015-04-23  1:55               ` Segher Boessenkool
  2015-04-23  3:17                 ` Peter Bergner
  0 siblings, 1 reply; 14+ messages in thread
From: Segher Boessenkool @ 2015-04-23  1:55 UTC (permalink / raw)
  To: Peter Bergner; +Cc: GCC Patches, David Edelsohn, Michael Meissner, Bill Schmidt

On Wed, Apr 22, 2015 at 06:08:26PM -0500, Peter Bergner wrote:
> > > > > +	  case HTM_BUILTIN_TTEST: /* Alias for: tabortwci. 0,r0,0  */
> > > > > +	    op[nopnds++] = GEN_INT (0);
> > > > > +	    op[nopnds++] = gen_rtx_REG (SImode, 0);
> > > > > +	    op[nopnds++] = GEN_INT (0);
> > > > 
> > > > Is that really r0, isn't that (0|rA)?  [Too lazy to read the docs myself
> > > > right now, sorry.]
> > > 
> > > The ISA doc shows:
> > 
> > [snip]
> > 
> > Thanks for looking it up!
> > 
> > I'm still a bit worried about putting a reg in the RTL (while the instruction
> > doesn't actually use one), but perhaps it's harmless.
> 
> I'm not sure what you mean by the "instruction doesn't use one".
> The hardware instruction does use a register for its second
> operand (even though its contents are ignored due to TO == 0)
> and the pattern requires us to pass in a reg rtx, so I'm not
> sure what you're referring to.

I mean the instruction doesn't actually use the value in the register
(if it did, you couldn't just pass in a non-fixed hard register in RTL).

Using a hard reg in the RTL like this has a few problems:
a) It might hinder register allocation.  Maybe it doesn't, not sure;
b) It does hinder scheduling;
c) It can make things ICE, maybe with register asm.

I no longer think c) will happen in this case.

The alternative is to write a separate define_insn for ttest, one
without inputs; the generated assembler can still be the same of
course.

Cheers,


Segher

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH, rs6000, testsuite] Fix PR target/64579, __TM_end __builtin_tend failed to return transactional state
  2015-04-23  1:55               ` Segher Boessenkool
@ 2015-04-23  3:17                 ` Peter Bergner
  0 siblings, 0 replies; 14+ messages in thread
From: Peter Bergner @ 2015-04-23  3:17 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: GCC Patches, David Edelsohn, Michael Meissner, Bill Schmidt

On Wed, 2015-04-22 at 20:55 -0500, Segher Boessenkool wrote:
> Using a hard reg in the RTL like this has a few problems:
> a) It might hinder register allocation.  Maybe it doesn't, not sure;
> b) It does hinder scheduling;
> c) It can make things ICE, maybe with register asm.

Ahh, I see what you mean now.  Yeah, I hadn't thought of that.


> The alternative is to write a separate define_insn for ttest, one
> without inputs; the generated assembler can still be the same of
> course.

In that case, I think you're right that this is the best course
if action.  I'll do that ans retest.  Thanks for catching this.

Peter


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH, rs6000, testsuite] Fix PR target/64579, __TM_end __builtin_tend failed to return transactional state
  2015-04-22 22:16           ` Segher Boessenkool
  2015-04-22 23:08             ` Peter Bergner
@ 2015-04-23 19:16             ` Peter Bergner
  2015-04-24 15:34               ` Segher Boessenkool
  1 sibling, 1 reply; 14+ messages in thread
From: Peter Bergner @ 2015-04-23 19:16 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: GCC Patches, David Edelsohn, Michael Meissner, Bill Schmidt

On Wed, 2015-04-22 at 17:16 -0500, Segher Boessenkool wrote:
> On Wed, Apr 22, 2015 at 08:43:10AM -0500, Peter Bergner wrote:
> > > Maybe you can fold tabortdc with tabortwc now?  Use one UNSPEC name
> > > for both, :GPR and <wd>?
> > 
> > Wouldn't that change the tabortwc pattern to use DImode rather
> > than SImode when compiled with -m64 or -m32 -mpowerpc64?
> > I'm not sure we want that.
> 
> The GPR mode iterator creates two patterns, one for SI and one for DI,
> and the tabortwc would be the one for SI if you use <wd>.

Ok, I combined the tabortdc and tabortwc patterns, as well as the
tabortdci and tabortwci patterns.  Thanks for the suggestion.



> I'm still a bit worried about putting a reg in the RTL (while the instruction
> doesn't actually use one), but perhaps it's harmless.

Ok, I created a separate ttest define_insn that hard codes the operands.
I switched to using r1 rather than r0 as the second operand, for the
reason that there could be code that sets r0 directly before this insn
and I didn't want to create a unneeded read-after-write dependency.
I thought r1 should be safe to use, since it's only updated in the
prologue/epilogue and should be constant for the life of the function.



> > > > +/* { dg-final { scan-assembler-times "tabortdci\\." 1 { target lp64 } } } */
> > > 
> > > This skips this test on -m32 -mpowerpc64, is that on purpose?
> > 
> > Ummm, not exactly.  :-)   Not that many people test that though.
> > I'll see if I can find a replacement for lp64 that covers that case.
> 
> Maybe just { powerpc64 } works?

powerpc64 doesn't work.  It tells us whether the target will execute
64-bit instructions or not.  When you run on a 64-bit host, then both
32-bit binaries and 64-bit binaries will gladly execute the 64-bit
category extsw instruction just fine.  This doesn't tell us that the
target has underlying 64-bit registers for its use or not.  Given that,
I think we should just leave this as lp64.

Updated patch below passed bootstrap and regtesting with no
regressions, so how does this look for trunk?

I'd also like to backport this to the open release branches so 
everything matches. Is that ok once I've verified bootstrap and
regtesting on each branch?


Peter



gcc/
	PR target/64579
	* config/rs6000/htm.md: Remove all define_expands.
	(UNSPECV_HTM_TABORTDC, UNSPECV_HTM_TABORTDCI, UNSPECV_HTM_TABORTWC,
	UNSPECV_HTM_TABORTWCI): Remove.
	(UNSPECV_HTM_TABORTXC, UNSPECV_HTM_TABORTXCI, UNSPECV_HTM_TTEST): New.
	(tabort_internal, tbegin_internal, tcheck_internal, tend_internal,
	trechkpt_internal, treclaim_internal, tsr_internal): Rename from this...
	(tabort, tbegin, tcheck, tend, trechkpt, treclaim, tsr): ...to this.
	(tabortdc_internal, tabortdci_internal, tabortwc_internal,
	tabortwci_internal): Remove define_insns.
	(tabort<wd>c, tabort<wd>ci): New define_insns.
	(tabort): Use gpc_reg_operand.
	(tcheck): Remove operand.
	(htm_mfspr_<mode>, htm_mtspr_<mode>): Use GPR mode macro.
	* config/rs6000/htmxlintrin.h (__TM_end): Use _HTM_TRANSACTIONAL as
	expected value.
	* config/rs6000/rs6000-builtin.def (BU_HTM_SPR0): Remove.
	(BU_HTM_SPR1): Rename to BU_HTM_V1.  Remove use of RS6000_BTC_SPR.
	(tabort, tabortdc, tabortdci, tabortwc, tabortwci, tbegin,
	tcheck, tend, tendall, trechkpt, treclaim, tresume, tsuspend,
	tsr, ttest): Pass in the RS6000_BTC_CR attribute.
	(get_tfhar, set_tfhar, get_tfiar, set_tfiar, get_texasr, set_texasr,
	get_texasru, set_texasru): Pass in the RS6000_BTC_SPR attribute.
	(tcheck): Remove builtin argument.
	* config/rs6000/rs6000.c (rs6000_htm_spr_icode): Use TARGET_POWERPC64
	not TARGET_64BIT.
	(htm_expand_builtin): Fix usage of expandedp.  Disallow usage of the
	tabortdc and tabortdci builtins when not in 64-bit mode.
	Modify code to handle the loss of the HTM define_expands.
	Emit code to copy the CR register to TARGET.
	(htm_init_builtins): Modify code to handle the loss of the HTM
	define_expands.
	* config/rs6000/rs6000.h (RS6000_BTC_32BIT): Delete.
	(RS6000_BTC_64BIT): Likewise.
	(RS6000_BTC_CR): New macro.
	* doc/extend.texi: Update documentation for htm builtins.

gcc/testsuite/

	PR target/64579
	* gcc.target/powerpc/htm-1.c: New test.
	* gcc.target/powerpc/htm-builtin-1.c (__builtin_tabortdc): Only test
	on 64-bit compiles.
	(__builtin_tabortdci): Likewise.
	(__builtin_tcheck): Remove operand.
	* lib/target-supports.exp (check_htm_hw_available): New function.


Index: gcc/config/rs6000/htm.md
===================================================================
--- gcc/config/rs6000/htm.md	(revision 222127)
+++ gcc/config/rs6000/htm.md	(working copy)
@@ -32,197 +32,52 @@ (define_constants
 
 (define_c_enum "unspecv"
   [UNSPECV_HTM_TABORT
-   UNSPECV_HTM_TABORTDC
-   UNSPECV_HTM_TABORTDCI
-   UNSPECV_HTM_TABORTWC
-   UNSPECV_HTM_TABORTWCI
+   UNSPECV_HTM_TABORTXC
+   UNSPECV_HTM_TABORTXCI
    UNSPECV_HTM_TBEGIN
    UNSPECV_HTM_TCHECK
    UNSPECV_HTM_TEND
    UNSPECV_HTM_TRECHKPT
    UNSPECV_HTM_TRECLAIM
    UNSPECV_HTM_TSR
+   UNSPECV_HTM_TTEST
    UNSPECV_HTM_MFSPR
    UNSPECV_HTM_MTSPR
   ])
 
 
-(define_expand "tabort"
-  [(set (match_dup 2)
-	(unspec_volatile:CC [(match_operand:SI 1 "int_reg_operand" "")]
-			    UNSPECV_HTM_TABORT))
-   (set (match_dup 3)
-	(eq:SI (match_dup 2)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 3)
-		(const_int 1)))]
-  "TARGET_HTM"
-{
-  operands[2] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[3] = gen_reg_rtx (SImode);
-})
-
-(define_insn "*tabort_internal"
+(define_insn "tabort"
   [(set (match_operand:CC 1 "cc_reg_operand" "=x")
-	(unspec_volatile:CC [(match_operand:SI 0 "int_reg_operand" "r")]
+	(unspec_volatile:CC [(match_operand:SI 0 "gpc_reg_operand" "r")]
 			    UNSPECV_HTM_TABORT))]
   "TARGET_HTM"
   "tabort. %0"
   [(set_attr "type" "htm")
    (set_attr "length" "4")])
 
-(define_expand "tabortdc"
-  [(set (match_dup 4)
-	(unspec_volatile:CC [(match_operand 1 "u5bit_cint_operand" "n")
-			     (match_operand:SI 2 "gpc_reg_operand" "r")
-			     (match_operand:SI 3 "gpc_reg_operand" "r")]
-			    UNSPECV_HTM_TABORTDC))
-   (set (match_dup 5)
-	(eq:SI (match_dup 4)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 5)
-		(const_int 1)))]
-  "TARGET_HTM"
-{
-  operands[4] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[5] = gen_reg_rtx (SImode);
-})
-
-(define_insn "*tabortdc_internal"
-  [(set (match_operand:CC 3 "cc_reg_operand" "=x")
-	(unspec_volatile:CC [(match_operand 0 "u5bit_cint_operand" "n")
-			     (match_operand:SI 1 "gpc_reg_operand" "r")
-			     (match_operand:SI 2 "gpc_reg_operand" "r")]
-			    UNSPECV_HTM_TABORTDC))]
-  "TARGET_HTM"
-  "tabortdc. %0,%1,%2"
-  [(set_attr "type" "htm")
-   (set_attr "length" "4")])
-
-(define_expand "tabortdci"
-  [(set (match_dup 4)
-	(unspec_volatile:CC [(match_operand 1 "u5bit_cint_operand" "n")
-			     (match_operand:SI 2 "gpc_reg_operand" "r")
-			     (match_operand 3 "s5bit_cint_operand" "n")]
-			    UNSPECV_HTM_TABORTDCI))
-   (set (match_dup 5)
-	(eq:SI (match_dup 4)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 5)
-		(const_int 1)))]
-  "TARGET_HTM"
-{
-  operands[4] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[5] = gen_reg_rtx (SImode);
-})
-
-(define_insn "*tabortdci_internal"
+(define_insn "tabort<wd>c"
   [(set (match_operand:CC 3 "cc_reg_operand" "=x")
 	(unspec_volatile:CC [(match_operand 0 "u5bit_cint_operand" "n")
-			     (match_operand:SI 1 "gpc_reg_operand" "r")
-			     (match_operand 2 "s5bit_cint_operand" "n")]
-			    UNSPECV_HTM_TABORTDCI))]
+			     (match_operand:GPR 1 "gpc_reg_operand" "r")
+			     (match_operand:GPR 2 "gpc_reg_operand" "r")]
+			    UNSPECV_HTM_TABORTXC))]
   "TARGET_HTM"
-  "tabortdci. %0,%1,%2"
-  [(set_attr "type" "htm")
-   (set_attr "length" "4")])
-
-(define_expand "tabortwc"
-  [(set (match_dup 4)
-	(unspec_volatile:CC [(match_operand 1 "u5bit_cint_operand" "n")
-			     (match_operand:SI 2 "gpc_reg_operand" "r")
-			     (match_operand:SI 3 "gpc_reg_operand" "r")]
-			    UNSPECV_HTM_TABORTWC))
-   (set (match_dup 5)
-	(eq:SI (match_dup 4)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 5)
-		(const_int 1)))]
-  "TARGET_HTM"
-{
-  operands[4] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[5] = gen_reg_rtx (SImode);
-})
-
-(define_insn "*tabortwc_internal"
-  [(set (match_operand:CC 3 "cc_reg_operand" "=x")
-	(unspec_volatile:CC [(match_operand 0 "u5bit_cint_operand" "n")
-			     (match_operand:SI 1 "gpc_reg_operand" "r")
-			     (match_operand:SI 2 "gpc_reg_operand" "r")]
-			    UNSPECV_HTM_TABORTWC))]
-  "TARGET_HTM"
-  "tabortwc. %0,%1,%2"
+  "tabort<wd>c. %0,%1,%2"
   [(set_attr "type" "htm")
    (set_attr "length" "4")])
 
-(define_expand "tabortwci"
-  [(set (match_dup 4)
-	(unspec_volatile:CC [(match_operand 1 "u5bit_cint_operand" "n")
-			     (match_operand:SI 2 "gpc_reg_operand" "r")
-			     (match_operand 3 "s5bit_cint_operand" "n")]
-			    UNSPECV_HTM_TABORTWCI))
-   (set (match_dup 5)
-	(eq:SI (match_dup 4)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 5)
-		(const_int 1)))]
-  "TARGET_HTM"
-{
-  operands[4] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[5] = gen_reg_rtx (SImode);
-})
-
-(define_expand "ttest"
-  [(set (match_dup 1)
-	(unspec_volatile:CC [(const_int 0)
-			     (reg:SI 0)
-			     (const_int 0)]
-			    UNSPECV_HTM_TABORTWCI))
-   (set (subreg:CC (match_dup 2) 0) (match_dup 1))
-   (set (match_dup 3) (lshiftrt:SI (match_dup 2) (const_int 28)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(and:SI (match_dup 3)
-		(const_int 15)))]
-  "TARGET_HTM"
-{
-  operands[1] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[2] = gen_reg_rtx (SImode);
-  operands[3] = gen_reg_rtx (SImode);
-})
-
-(define_insn "*tabortwci_internal"
+(define_insn "tabort<wd>ci"
   [(set (match_operand:CC 3 "cc_reg_operand" "=x")
 	(unspec_volatile:CC [(match_operand 0 "u5bit_cint_operand" "n")
-			     (match_operand:SI 1 "gpc_reg_operand" "r")
+			     (match_operand:GPR 1 "gpc_reg_operand" "r")
 			     (match_operand 2 "s5bit_cint_operand" "n")]
-			    UNSPECV_HTM_TABORTWCI))]
+			    UNSPECV_HTM_TABORTXCI))]
   "TARGET_HTM"
-  "tabortwci. %0,%1,%2"
+  "tabort<wd>ci. %0,%1,%2"
   [(set_attr "type" "htm")
    (set_attr "length" "4")])
 
-(define_expand "tbegin"
-  [(set (match_dup 2)
-	(unspec_volatile:CC [(match_operand 1 "const_0_to_1_operand" "n")]
-			    UNSPECV_HTM_TBEGIN))
-   (set (match_dup 3)
-	(eq:SI (match_dup 2)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 3)
-		(const_int 1)))]
-  "TARGET_HTM"
-{
-  operands[2] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[3] = gen_reg_rtx (SImode);
-})
-
-(define_insn "*tbegin_internal"
+(define_insn "tbegin"
   [(set (match_operand:CC 1 "cc_reg_operand" "=x")
 	(unspec_volatile:CC [(match_operand 0 "const_0_to_1_operand" "n")]
 			    UNSPECV_HTM_TBEGIN))]
@@ -231,48 +86,16 @@ (define_insn "*tbegin_internal"
   [(set_attr "type" "htm")
    (set_attr "length" "4")])
 
-(define_expand "tcheck"
-  [(set (match_dup 2)
-	(unspec_volatile:CC [(match_operand 1 "u3bit_cint_operand" "n")]
-			    UNSPECV_HTM_TCHECK))
-   (set (match_dup 3)
-	(eq:SI (match_dup 2)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 3)
-		(const_int 1)))]
-  "TARGET_HTM"
-{
-  operands[2] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[3] = gen_reg_rtx (SImode);
-})
-
-(define_insn "*tcheck_internal"
-  [(set (match_operand:CC 1 "cc_reg_operand" "=x")
-	(unspec_volatile:CC [(match_operand 0 "u3bit_cint_operand" "n")]
+(define_insn "tcheck"
+  [(set (match_operand:CC 0 "cc_reg_operand" "=y")
+	(unspec_volatile:CC [(const_int 0)]
 			    UNSPECV_HTM_TCHECK))]
   "TARGET_HTM"
   "tcheck %0"
   [(set_attr "type" "htm")
    (set_attr "length" "4")])
 
-(define_expand "tend"
-  [(set (match_dup 2)
-	(unspec_volatile:CC [(match_operand 1 "const_0_to_1_operand" "n")]
-			    UNSPECV_HTM_TEND))
-   (set (match_dup 3)
-	(eq:SI (match_dup 2)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 3)
-		(const_int 1)))]
-  "TARGET_HTM"
-{
-  operands[2] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[3] = gen_reg_rtx (SImode);
-})
-
-(define_insn "*tend_internal"
+(define_insn "tend"
   [(set (match_operand:CC 1 "cc_reg_operand" "=x")
 	(unspec_volatile:CC [(match_operand 0 "const_0_to_1_operand" "n")]
 			    UNSPECV_HTM_TEND))]
@@ -281,23 +104,7 @@ (define_insn "*tend_internal"
   [(set_attr "type" "htm")
    (set_attr "length" "4")])
 
-(define_expand "trechkpt"
-  [(set (match_dup 1)
-	(unspec_volatile:CC [(const_int 0)]
-			    UNSPECV_HTM_TRECHKPT))
-   (set (match_dup 2)
-	(eq:SI (match_dup 1)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 2)
-		(const_int 1)))]
-  "TARGET_HTM"
-{
-  operands[1] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[2] = gen_reg_rtx (SImode);
-})
-
-(define_insn "*trechkpt_internal"
+(define_insn "trechkpt"
   [(set (match_operand:CC 0 "cc_reg_operand" "=x")
 	(unspec_volatile:CC [(const_int 0)]
 			    UNSPECV_HTM_TRECHKPT))]
@@ -306,23 +113,7 @@ (define_insn "*trechkpt_internal"
   [(set_attr "type" "htm")
    (set_attr "length" "4")])
 
-(define_expand "treclaim"
-  [(set (match_dup 2)
-	(unspec_volatile:CC [(match_operand:SI 1 "gpc_reg_operand" "r")]
-			    UNSPECV_HTM_TRECLAIM))
-   (set (match_dup 3)
-	(eq:SI (match_dup 2)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 3)
-		(const_int 1)))]
-  "TARGET_HTM"
-{
-  operands[2] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[3] = gen_reg_rtx (SImode);
-})
-
-(define_insn "*treclaim_internal"
+(define_insn "treclaim"
   [(set (match_operand:CC 1 "cc_reg_operand" "=x")
 	(unspec_volatile:CC [(match_operand:SI 0 "gpc_reg_operand" "r")]
 			    UNSPECV_HTM_TRECLAIM))]
@@ -331,23 +122,7 @@ (define_insn "*treclaim_internal"
   [(set_attr "type" "htm")
    (set_attr "length" "4")])
 
-(define_expand "tsr"
-  [(set (match_dup 2)
-	(unspec_volatile:CC [(match_operand 1 "const_0_to_1_operand" "n")]
-			    UNSPECV_HTM_TSR))
-   (set (match_dup 3)
-	(eq:SI (match_dup 2)
-	       (const_int 0)))
-   (set (match_operand:SI 0 "int_reg_operand" "")
-	(xor:SI (match_dup 3)
-		(const_int 1)))]
-  "TARGET_HTM"
-{
-  operands[2] = gen_rtx_REG (CCmode, CR0_REGNO);
-  operands[3] = gen_reg_rtx (SImode);
-})
-
-(define_insn "*tsr_internal"
+(define_insn "tsr"
   [(set (match_operand:CC 1 "cc_reg_operand" "=x")
 	(unspec_volatile:CC [(match_operand 0 "const_0_to_1_operand" "n")]
 			    UNSPECV_HTM_TSR))]
@@ -356,21 +131,30 @@ (define_insn "*tsr_internal"
   [(set_attr "type" "htm")
    (set_attr "length" "4")])
 
+(define_insn "ttest"
+  [(set (match_operand:CC 0 "cc_reg_operand" "=x")
+	(unspec_volatile:CC [(const_int 0)]
+			    UNSPECV_HTM_TTEST))]
+  "TARGET_HTM"
+  "tabortwci. 0,1,0"
+  [(set_attr "type" "htm")
+   (set_attr "length" "4")])
+
 (define_insn "htm_mfspr_<mode>"
-  [(set (match_operand:P 0 "gpc_reg_operand" "=r")
-        (unspec_volatile:P [(match_operand 1 "u10bit_cint_operand" "n")
-			    (match_operand:P 2 "htm_spr_reg_operand" "")]
-			   UNSPECV_HTM_MFSPR))]
+  [(set (match_operand:GPR 0 "gpc_reg_operand" "=r")
+        (unspec_volatile:GPR [(match_operand 1 "u10bit_cint_operand" "n")
+			      (match_operand:GPR 2 "htm_spr_reg_operand" "")]
+			     UNSPECV_HTM_MFSPR))]
   "TARGET_HTM"
   "mfspr %0,%1";
   [(set_attr "type" "htm")
    (set_attr "length" "4")])
 
 (define_insn "htm_mtspr_<mode>"
-  [(set (match_operand:P 2 "htm_spr_reg_operand" "")
-        (unspec_volatile:P [(match_operand:P 0 "gpc_reg_operand" "r")
-			    (match_operand 1 "u10bit_cint_operand" "n")]
-                           UNSPECV_HTM_MTSPR))]
+  [(set (match_operand:GPR 2 "htm_spr_reg_operand" "")
+        (unspec_volatile:GPR [(match_operand:GPR 0 "gpc_reg_operand" "r")
+			      (match_operand 1 "u10bit_cint_operand" "n")]
+			     UNSPECV_HTM_MTSPR))]
   "TARGET_HTM"
   "mtspr %1,%0";
   [(set_attr "type" "htm")
Index: gcc/config/rs6000/htmxlintrin.h
===================================================================
--- gcc/config/rs6000/htmxlintrin.h	(revision 222127)
+++ gcc/config/rs6000/htmxlintrin.h	(working copy)
@@ -81,7 +81,8 @@ extern __inline long
 __attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
 __TM_end (void)
 {
-  if (__builtin_expect (__builtin_tend (0), 1))
+  unsigned char status = _HTM_STATE (__builtin_tend (0));
+  if (__builtin_expect (status, _HTM_TRANSACTIONAL))
     return 1;
   return 0;
 }
Index: gcc/config/rs6000/rs6000-builtin.def
===================================================================
--- gcc/config/rs6000/rs6000-builtin.def	(revision 222127)
+++ gcc/config/rs6000/rs6000-builtin.def	(working copy)
@@ -456,21 +456,12 @@
 		     | RS6000_BTC_TERNARY),				\
 		    CODE_FOR_ ## ICODE)			/* ICODE */
 
-#define BU_HTM_SPR0(ENUM, NAME, ATTR, ICODE)				\
-  RS6000_BUILTIN_H (HTM_BUILTIN_ ## ENUM,		/* ENUM */	\
-		    "__builtin_" NAME,			/* NAME */	\
-		    RS6000_BTM_HTM,			/* MASK */	\
-		    (RS6000_BTC_ ## ATTR		/* ATTR */	\
-		     | RS6000_BTC_SPR),					\
-		    CODE_FOR_ ## ICODE)			/* ICODE */
-
-#define BU_HTM_SPR1(ENUM, NAME, ATTR, ICODE)				\
+#define BU_HTM_V1(ENUM, NAME, ATTR, ICODE)				\
   RS6000_BUILTIN_H (HTM_BUILTIN_ ## ENUM,		/* ENUM */	\
 		    "__builtin_" NAME,			/* NAME */	\
 		    RS6000_BTM_HTM,			/* MASK */	\
 		    (RS6000_BTC_ ## ATTR		/* ATTR */	\
 		     | RS6000_BTC_UNARY					\
-		     | RS6000_BTC_SPR					\
 		     | RS6000_BTC_VOID),				\
 		    CODE_FOR_ ## ICODE)			/* ICODE */
 
@@ -1633,30 +1624,30 @@ BU_CRYPTO_OVERLOAD_3 (VSHASIGMA, "vshasi
 
 \f
 /* HTM functions.  */
-BU_HTM_1  (TABORT,	"tabort",	MISC,	tabort)
-BU_HTM_3  (TABORTDC,	"tabortdc",	MISC,	tabortdc)
-BU_HTM_3  (TABORTDCI,	"tabortdci",	MISC,	tabortdci)
-BU_HTM_3  (TABORTWC,	"tabortwc",	MISC,	tabortwc)
-BU_HTM_3  (TABORTWCI,	"tabortwci",	MISC,	tabortwci)
-BU_HTM_1  (TBEGIN,	"tbegin",	MISC,	tbegin)
-BU_HTM_1  (TCHECK,	"tcheck",	MISC,	tcheck)
-BU_HTM_1  (TEND,	"tend",		MISC,	tend)
-BU_HTM_0  (TENDALL,	"tendall",	MISC,	tend)
-BU_HTM_0  (TRECHKPT,	"trechkpt",	MISC,	trechkpt)
-BU_HTM_1  (TRECLAIM,	"treclaim",	MISC,	treclaim)
-BU_HTM_0  (TRESUME,	"tresume",	MISC,	tsr)
-BU_HTM_0  (TSUSPEND,	"tsuspend",	MISC,	tsr)
-BU_HTM_1  (TSR,		"tsr",		MISC,	tsr)
-BU_HTM_0  (TTEST,	"ttest",	MISC,	ttest)
-
-BU_HTM_SPR0 (GET_TFHAR,		"get_tfhar",	MISC,	nothing)
-BU_HTM_SPR1 (SET_TFHAR,		"set_tfhar",	MISC,	nothing)
-BU_HTM_SPR0 (GET_TFIAR,		"get_tfiar",	MISC,	nothing)
-BU_HTM_SPR1 (SET_TFIAR,		"set_tfiar",	MISC,	nothing)
-BU_HTM_SPR0 (GET_TEXASR,	"get_texasr",	MISC,	nothing)
-BU_HTM_SPR1 (SET_TEXASR,	"set_texasr",	MISC,	nothing)
-BU_HTM_SPR0 (GET_TEXASRU,	"get_texasru",	MISC,	nothing)
-BU_HTM_SPR1 (SET_TEXASRU,	"set_texasru",	MISC,	nothing)
+BU_HTM_1  (TABORT,	"tabort",	CR,	tabort)
+BU_HTM_3  (TABORTDC,	"tabortdc",	CR,	tabortdc)
+BU_HTM_3  (TABORTDCI,	"tabortdci",	CR,	tabortdci)
+BU_HTM_3  (TABORTWC,	"tabortwc",	CR,	tabortwc)
+BU_HTM_3  (TABORTWCI,	"tabortwci",	CR,	tabortwci)
+BU_HTM_1  (TBEGIN,	"tbegin",	CR,	tbegin)
+BU_HTM_0  (TCHECK,	"tcheck",	CR,	tcheck)
+BU_HTM_1  (TEND,	"tend",		CR,	tend)
+BU_HTM_0  (TENDALL,	"tendall",	CR,	tend)
+BU_HTM_0  (TRECHKPT,	"trechkpt",	CR,	trechkpt)
+BU_HTM_1  (TRECLAIM,	"treclaim",	CR,	treclaim)
+BU_HTM_0  (TRESUME,	"tresume",	CR,	tsr)
+BU_HTM_0  (TSUSPEND,	"tsuspend",	CR,	tsr)
+BU_HTM_1  (TSR,		"tsr",		CR,	tsr)
+BU_HTM_0  (TTEST,	"ttest",	CR,	ttest)
+
+BU_HTM_0  (GET_TFHAR,	"get_tfhar",	SPR,	nothing)
+BU_HTM_V1 (SET_TFHAR,	"set_tfhar",	SPR,	nothing)
+BU_HTM_0  (GET_TFIAR,	"get_tfiar",	SPR,	nothing)
+BU_HTM_V1 (SET_TFIAR,	"set_tfiar",	SPR,	nothing)
+BU_HTM_0  (GET_TEXASR,	"get_texasr",	SPR,	nothing)
+BU_HTM_V1 (SET_TEXASR,	"set_texasr",	SPR,	nothing)
+BU_HTM_0  (GET_TEXASRU,	"get_texasru",	SPR,	nothing)
+BU_HTM_V1 (SET_TEXASRU,	"set_texasru",	SPR,	nothing)
 
 \f
 /* 3 argument paired floating point builtins.  */
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 222127)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -12655,9 +12655,9 @@ static inline enum insn_code
 rs6000_htm_spr_icode (bool nonvoid)
 {
   if (nonvoid)
-    return (TARGET_64BIT) ? CODE_FOR_htm_mfspr_di : CODE_FOR_htm_mfspr_si;
+    return (TARGET_POWERPC64) ? CODE_FOR_htm_mfspr_di : CODE_FOR_htm_mfspr_si;
   else
-    return (TARGET_64BIT) ? CODE_FOR_htm_mtspr_di : CODE_FOR_htm_mtspr_si;
+    return (TARGET_POWERPC64) ? CODE_FOR_htm_mtspr_di : CODE_FOR_htm_mtspr_si;
 }
 
 /* Expand the HTM builtin in EXP and store the result in TARGET.
@@ -12671,7 +12671,17 @@ htm_expand_builtin (tree exp, rtx target
   const struct builtin_description *d;
   size_t i;
 
-  *expandedp = false;
+  *expandedp = true;
+
+  if (!TARGET_POWERPC64
+      && (fcode == HTM_BUILTIN_TABORTDC
+	  || fcode == HTM_BUILTIN_TABORTDCI))
+    {
+      size_t uns_fcode = (size_t)fcode;
+      const char *name = rs6000_builtin_info[uns_fcode].name;
+      error ("builtin %s is only valid in 64-bit mode", name);
+      return const0_rtx;
+    }
 
   /* Expand the HTM builtins.  */
   d = bdesc_htm;
@@ -12684,26 +12694,29 @@ htm_expand_builtin (tree exp, rtx target
 	call_expr_arg_iterator iter;
 	unsigned attr = rs6000_builtin_info[fcode].attr;
 	enum insn_code icode = d->icode;
+	const struct insn_operand_data *insn_op;
+	bool uses_spr = (attr & RS6000_BTC_SPR);
+	rtx cr = NULL_RTX;
 
-	if (attr & RS6000_BTC_SPR)
+	if (uses_spr)
 	  icode = rs6000_htm_spr_icode (nonvoid);
+	insn_op = &insn_data[icode].operand[0];
 
 	if (nonvoid)
 	  {
-	    machine_mode tmode = insn_data[icode].operand[0].mode;
+	    machine_mode tmode = (uses_spr) ? insn_op->mode : SImode;
 	    if (!target
 		|| GET_MODE (target) != tmode
-		|| !(*insn_data[icode].operand[0].predicate) (target, tmode))
+		|| (uses_spr && !(*insn_op->predicate) (target, tmode)))
 	      target = gen_reg_rtx (tmode);
-	    op[nopnds++] = target;
+	    if (uses_spr)
+	      op[nopnds++] = target;
 	  }
 
 	FOR_EACH_CALL_EXPR_ARG (arg, iter, exp)
 	{
-	  const struct insn_operand_data *insn_op;
-
 	  if (arg == error_mark_node || nopnds >= MAX_HTM_OPERANDS)
-	    return NULL_RTX;
+	    return const0_rtx;
 
 	  insn_op = &insn_data[icode].operand[nopnds];
 
@@ -12750,10 +12763,17 @@ htm_expand_builtin (tree exp, rtx target
 
 	/* If this builtin accesses SPRs, then pass in the appropriate
 	   SPR number and SPR regno as the last two operands.  */
-	if (attr & RS6000_BTC_SPR)
+	if (uses_spr)
 	  {
-	    op[nopnds++] = gen_rtx_CONST_INT (Pmode, htm_spr_num (fcode));
-	    op[nopnds++] = gen_rtx_REG (Pmode, htm_spr_regno (fcode));
+	    machine_mode mode = (TARGET_POWERPC64) ? DImode : SImode;
+	    op[nopnds++] = gen_rtx_CONST_INT (mode, htm_spr_num (fcode));
+	    op[nopnds++] = gen_rtx_REG (mode, htm_spr_regno (fcode));
+	  }
+	/* If this builtin accesses a CR, then pass in a scratch
+	   CR as the last operand.  */
+	else if (attr & RS6000_BTC_CR)
+	  { cr = gen_reg_rtx (CCmode);
+	    op[nopnds++] = cr;
 	  }
 
 #ifdef ENABLE_CHECKING
@@ -12766,7 +12786,7 @@ htm_expand_builtin (tree exp, rtx target
 	  expected_nopnds = 3;
 	if (!(attr & RS6000_BTC_VOID))
 	  expected_nopnds += 1;
-	if (attr & RS6000_BTC_SPR)
+	if (uses_spr)
 	  expected_nopnds += 2;
 
 	gcc_assert (nopnds == expected_nopnds && nopnds <= MAX_HTM_OPERANDS);
@@ -12793,12 +12813,41 @@ htm_expand_builtin (tree exp, rtx target
 	  return NULL_RTX;
 	emit_insn (pat);
 
-	*expandedp = true;
+	if (attr & RS6000_BTC_CR)
+	  {
+	    if (fcode == HTM_BUILTIN_TBEGIN)
+	      {
+		/* Emit code to set TARGET to true or false depending on
+		   whether the tbegin. instruction successfully or failed
+		   to start a transaction.  We do this by placing the 1's
+		   complement of CR's EQ bit into TARGET.  */
+		rtx scratch = gen_reg_rtx (SImode);
+		emit_insn (gen_rtx_SET (VOIDmode, scratch,
+					gen_rtx_EQ (SImode, cr,
+						     const0_rtx)));
+		emit_insn (gen_rtx_SET (VOIDmode, target,
+					gen_rtx_XOR (SImode, scratch,
+						     GEN_INT (1))));
+	      }
+	    else
+	      {
+		/* Emit code to copy the 4-bit condition register field
+		   CR into the least significant end of register TARGET.  */
+		rtx scratch1 = gen_reg_rtx (SImode);
+		rtx scratch2 = gen_reg_rtx (SImode);
+		rtx subreg = simplify_gen_subreg (CCmode, scratch1, SImode, 0);
+		emit_insn (gen_movcc (subreg, cr));
+		emit_insn (gen_lshrsi3 (scratch2, scratch1, GEN_INT (28)));
+		emit_insn (gen_andsi3 (target, scratch2, GEN_INT (0xf)));
+	      }
+	  }
+
 	if (nonvoid)
 	  return target;
 	return const0_rtx;
       }
 
+  *expandedp = false;
   return NULL_RTX;
 }
 
@@ -15287,8 +15336,31 @@ htm_init_builtins (void)
       bool void_func = (attr & RS6000_BTC_VOID);
       int attr_args = (attr & RS6000_BTC_TYPE_MASK);
       int nopnds = 0;
-      tree argtype = (attr & RS6000_BTC_SPR) ? long_unsigned_type_node
-					     : unsigned_type_node;
+      tree gpr_type_node;
+      tree rettype;
+      tree argtype;
+
+      if (TARGET_32BIT && TARGET_POWERPC64)
+	gpr_type_node = long_long_unsigned_type_node;
+      else
+	gpr_type_node = long_unsigned_type_node;
+
+      if (attr & RS6000_BTC_SPR)
+	{
+	  rettype = gpr_type_node;
+	  argtype = gpr_type_node;
+	}
+      else if (d->code == HTM_BUILTIN_TABORTDC
+	       || d->code == HTM_BUILTIN_TABORTDCI)
+	{
+	  rettype = unsigned_type_node;
+	  argtype = gpr_type_node;
+	}
+      else
+	{
+	  rettype = unsigned_type_node;
+	  argtype = unsigned_type_node;
+	}
 
       if ((mask & builtin_mask) != mask)
 	{
@@ -15305,7 +15377,7 @@ htm_init_builtins (void)
 	  continue;
 	}
 
-      op[nopnds++] = (void_func) ? void_type_node : argtype;
+      op[nopnds++] = (void_func) ? void_type_node : rettype;
 
       if (attr_args == RS6000_BTC_UNARY)
 	op[nopnds++] = argtype;
Index: gcc/config/rs6000/rs6000.h
===================================================================
--- gcc/config/rs6000/rs6000.h	(revision 222127)
+++ gcc/config/rs6000/rs6000.h	(working copy)
@@ -2573,9 +2573,8 @@ extern int frame_pointer_needed;
 /* Miscellaneous information.  */
 #define RS6000_BTC_SPR		0x01000000	/* function references SPRs.  */
 #define RS6000_BTC_VOID		0x02000000	/* function has no return value.  */
-#define RS6000_BTC_OVERLOADED	0x04000000	/* function is overloaded.  */
-#define RS6000_BTC_32BIT	0x08000000	/* function references SPRs.  */
-#define RS6000_BTC_64BIT	0x10000000	/* function references SPRs.  */
+#define RS6000_BTC_CR		0x04000000	/* function references a CR.  */
+#define RS6000_BTC_OVERLOADED	0x08000000	/* function is overloaded.  */
 #define RS6000_BTC_MISC_MASK	0x1f000000	/* Mask of the misc info.  */
 
 /* Convenience macros to document the instruction type.  */
Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 222127)
+++ gcc/doc/extend.texi	(working copy)
@@ -15121,10 +15121,15 @@ The following low level built-in functio
 @option{-mhtm} or @option{-mcpu=CPU} where CPU is `power8' or later.
 They all generate the machine instruction that is part of the name.
 
-The HTM built-ins return true or false depending on their success and
-their arguments match exactly the type and order of the associated
-hardware instruction's operands.  Refer to the ISA manual for a
-description of each instruction's operands.
+The HTM builtins (with the exception of @code{__builtin_tbegin}) return
+the full 4-bit condition register value set by their associated hardware
+instruction.  The header file @code{htmintrin.h} defines some macros that can
+be used to decipher the return value.  The @code{__builtin_tbegin} builtin
+returns a simple true or false value depending on whether a transaction was
+successfully started or not.  The arguments of the builtins match exactly the
+type and order of the associated hardware instruction's operands, except for
+the @code{__builtin_tcheck} builtin, which does not take any input arguments.
+Refer to the ISA manual for a description of each instruction's operands.
 
 @smallexample
 unsigned int __builtin_tbegin (unsigned int)
@@ -15136,7 +15141,7 @@ unsigned int __builtin_tabortdci (unsign
 unsigned int __builtin_tabortwc (unsigned int, unsigned int, unsigned int)
 unsigned int __builtin_tabortwci (unsigned int, unsigned int, int)
 
-unsigned int __builtin_tcheck (unsigned int)
+unsigned int __builtin_tcheck (void)
 unsigned int __builtin_treclaim (unsigned int)
 unsigned int __builtin_trechkpt (void)
 unsigned int __builtin_tsr (unsigned int)
@@ -15271,7 +15276,7 @@ TM_buff_type TM_buff;
 
 while (1)
   @{
-    if (__TM_begin (TM_buff))
+    if (__TM_begin (TM_buff) == _HTM_TBEGIN_STARTED)
       @{
         /* Transaction State Initiated.  */
         if (is_locked (lock))
Index: gcc/testsuite/gcc.target/powerpc/htm-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/htm-1.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/htm-1.c	(working copy)
@@ -0,0 +1,52 @@
+/* { dg-do run { target { powerpc*-*-* && htm_hw } } } */
+/* { dg-require-effective-target powerpc_htm_ok } */
+/* { dg-options "-mhtm" } */
+
+/* Program to test PowerPC HTM instructions.  */
+
+#include <stdlib.h>
+#include <htmintrin.h>
+
+int
+main (void)
+{
+  long i;
+  unsigned long mask = 0;
+
+repeat:
+  if (__builtin_tbegin (0))
+    {
+      mask++;
+    }
+  else
+    abort();
+
+  if (mask == 1)
+    {
+      __builtin_tsuspend ();
+
+      if (_HTM_STATE (__builtin_tcheck ()) != _HTM_SUSPENDED)
+	abort ();
+
+      __builtin_tresume ();
+
+      if (_HTM_STATE (__builtin_tcheck ()) != _HTM_TRANSACTIONAL)
+	abort ();
+    }
+  else
+    mask++;
+
+  if (_HTM_STATE (__builtin_tendall ()) != _HTM_TRANSACTIONAL)
+    abort ();
+
+  if (mask == 1)
+    goto repeat;
+
+  if (_HTM_STATE (__builtin_tendall ()) != _HTM_NONTRANSACTIONAL)
+    abort ();
+
+  if (mask != 3)
+    abort ();
+
+  return 0;
+}
Index: gcc/testsuite/gcc.target/powerpc/htm-builtin-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/htm-builtin-1.c	(revision 222127)
+++ gcc/testsuite/gcc.target/powerpc/htm-builtin-1.c	(working copy)
@@ -6,8 +6,8 @@
 /* { dg-final { scan-assembler-times "tbegin\\." 1 } } */
 /* { dg-final { scan-assembler-times "tend\\." 2 } } */
 /* { dg-final { scan-assembler-times "tabort\\." 2 } } */
-/* { dg-final { scan-assembler-times "tabortdc\\." 1 } } */
-/* { dg-final { scan-assembler-times "tabortdci\\." 1 } } */
+/* { dg-final { scan-assembler-times "tabortdc\\." 1 { target lp64 } } } */
+/* { dg-final { scan-assembler-times "tabortdci\\." 1 { target lp64 } } } */
 /* { dg-final { scan-assembler-times "tabortwc\\." 1 } } */
 /* { dg-final { scan-assembler-times "tabortwci\\." 2 } } */
 /* { dg-final { scan-assembler-times "tcheck" 1 } } */
@@ -25,12 +25,14 @@ void use_builtins (long *p, char code, l
   p[3] = __builtin_tabort (0);
   p[4] = __builtin_tabort (code);
 
+#ifdef __powerpc64__
   p[5] = __builtin_tabortdc (0xf, a[5], b[5]);
   p[6] = __builtin_tabortdci (0xf, a[6], 13);
+#endif
   p[7] = __builtin_tabortwc (0xf, a[7], b[7]);
   p[8] = __builtin_tabortwci (0xf, a[8], 13);
 
-  p[9] = __builtin_tcheck (5);
+  p[9] = __builtin_tcheck ();
   p[10] = __builtin_trechkpt ();
   p[11] = __builtin_treclaim (0);
   p[12] = __builtin_tresume ();
Index: gcc/testsuite/lib/target-supports.exp
===================================================================
--- gcc/testsuite/lib/target-supports.exp	(revision 222127)
+++ gcc/testsuite/lib/target-supports.exp	(working copy)
@@ -3279,6 +3279,25 @@ proc check_effective_target_powerpc_htm_
     }
 }
 
+# Return 1 if the target supports executing HTM hardware instructions,
+# 0 otherwise.  Cache the result.
+
+proc check_htm_hw_available { } {
+    return [check_cached_effective_target htm_hw_available {
+	# For now, disable on Darwin
+	if { [istarget powerpc-*-eabi] || [istarget powerpc*-*-eabispe] || [istarget *-*-darwin*]} {
+	    expr 0
+	} else {
+	    check_runtime_nocache htm_hw_available {
+		int main()
+		{
+		  __builtin_ttest ();
+		  return 0;
+		}
+	    } "-mhtm"
+	}
+    }]
+}
 # Return 1 if this is a PowerPC target supporting -mcpu=cell.
 
 proc check_effective_target_powerpc_ppu_ok { } {
@@ -5278,6 +5297,7 @@ proc is-effective-target { arg } {
 	  "p8vector_hw"    { set selected [check_p8vector_hw_available] }
 	  "ppc_recip_hw"   { set selected [check_ppc_recip_hw_available] }
 	  "dfp_hw"         { set selected [check_dfp_hw_available] }
+	  "htm_hw"         { set selected [check_htm_hw_available] }
 	  "named_sections" { set selected [check_named_sections_available] }
 	  "gc_sections"    { set selected [check_gc_sections_available] }
 	  "cxa_atexit"     { set selected [check_cxa_atexit_available] }
@@ -5301,6 +5321,7 @@ proc is-effective-target-keyword { arg }
 	  "p8vector_hw"    { return 1 }
 	  "ppc_recip_hw"   { return 1 }
 	  "dfp_hw"         { return 1 }
+	  "htm_hw"         { return 1 }
 	  "named_sections" { return 1 }
 	  "gc_sections"    { return 1 }
 	  "cxa_atexit"     { return 1 }




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH, rs6000, testsuite] Fix PR target/64579, __TM_end __builtin_tend failed to return transactional state
  2015-04-23 19:16             ` Peter Bergner
@ 2015-04-24 15:34               ` Segher Boessenkool
  2015-04-24 15:40                 ` David Edelsohn
  0 siblings, 1 reply; 14+ messages in thread
From: Segher Boessenkool @ 2015-04-24 15:34 UTC (permalink / raw)
  To: Peter Bergner; +Cc: GCC Patches, David Edelsohn, Michael Meissner, Bill Schmidt

On Thu, Apr 23, 2015 at 02:16:04PM -0500, Peter Bergner wrote:
> Ok, I created a separate ttest define_insn that hard codes the operands.
> I switched to using r1 rather than r0 as the second operand, for the
> reason that there could be code that sets r0 directly before this insn
> and I didn't want to create a unneeded read-after-write dependency.
> I thought r1 should be safe to use, since it's only updated in the
> prologue/epilogue and should be constant for the life of the function.

GPR1 is very likely already read before as well :-)

> > Maybe just { powerpc64 } works?
> 
> powerpc64 doesn't work.  It tells us whether the target will execute
> 64-bit instructions or not.

Ah yes, it is more like a "powerpc64_hw".  Oh well.

All looks great to me now, thanks for the changes,


Segher

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH, rs6000, testsuite] Fix PR target/64579, __TM_end __builtin_tend failed to return transactional state
  2015-04-24 15:34               ` Segher Boessenkool
@ 2015-04-24 15:40                 ` David Edelsohn
  2015-04-27 15:21                   ` Peter Bergner
  0 siblings, 1 reply; 14+ messages in thread
From: David Edelsohn @ 2015-04-24 15:40 UTC (permalink / raw)
  To: Segher Boessenkool, Peter Bergner
  Cc: GCC Patches, Michael Meissner, Bill Schmidt

On Fri, Apr 24, 2015 at 11:34 AM, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
> On Thu, Apr 23, 2015 at 02:16:04PM -0500, Peter Bergner wrote:
>> Ok, I created a separate ttest define_insn that hard codes the operands.
>> I switched to using r1 rather than r0 as the second operand, for the
>> reason that there could be code that sets r0 directly before this insn
>> and I didn't want to create a unneeded read-after-write dependency.
>> I thought r1 should be safe to use, since it's only updated in the
>> prologue/epilogue and should be constant for the life of the function.
>
> GPR1 is very likely already read before as well :-)
>
>> > Maybe just { powerpc64 } works?
>>
>> powerpc64 doesn't work.  It tells us whether the target will execute
>> 64-bit instructions or not.
>
> Ah yes, it is more like a "powerpc64_hw".  Oh well.
>
> All looks great to me now, thanks for the changes,

Okay for me.

Thanks, David

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH, rs6000, testsuite] Fix PR target/64579, __TM_end __builtin_tend failed to return transactional state
  2015-04-24 15:40                 ` David Edelsohn
@ 2015-04-27 15:21                   ` Peter Bergner
  0 siblings, 0 replies; 14+ messages in thread
From: Peter Bergner @ 2015-04-27 15:21 UTC (permalink / raw)
  To: David Edelsohn; +Cc: Segher Boessenkool, GCC Patches

On Fri, 2015-04-24 at 11:40 -0400, David Edelsohn wrote:
> On Fri, Apr 24, 2015 at 11:34 AM, Segher Boessenkool
> > All looks great to me now, thanks for the changes,
> 
> Okay for me.

In back porting my patch, I see that the patch doesn't apply
cleanly to the 4.8/4.9 branch code.  Looking at why, I see that it's
because the 4.8/4.9 code doesn't have the following change from
Segher.  I think we want this patch back ported too, don't we?

2014-09-11  Segher Boessenkool  <segher@kernel.crashing.org>

       * config/rs6000/htm.md (tabort, tabortdc, tabortdci, tabortwc,
       tabortwci, tbegin, tcheck, tend, trechkpt, treclaim, tsr): Use xor
       instead of minus.
       * config/rs6000/vector.md (cr6_test_for_zero_reverse,
       cr6_test_for_lt_reverse): Ditto.

If so, I can back port it and test it along with mine.  Otherwise, I'll
just work around it.  Let me know what you want me to do.

Peter


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2015-04-27 15:21 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-20 19:27 [PATCH, rs6000, testsuite] Fix PR target/64579, __TM_end __builtin_tend failed to return transactional state Peter Bergner
2015-03-20 20:52 ` Segher Boessenkool
2015-03-20 22:42   ` Peter Bergner
2015-04-21 20:56     ` Peter Bergner
2015-04-22  2:18       ` Segher Boessenkool
2015-04-22 13:43         ` Peter Bergner
2015-04-22 22:16           ` Segher Boessenkool
2015-04-22 23:08             ` Peter Bergner
2015-04-23  1:55               ` Segher Boessenkool
2015-04-23  3:17                 ` Peter Bergner
2015-04-23 19:16             ` Peter Bergner
2015-04-24 15:34               ` Segher Boessenkool
2015-04-24 15:40                 ` David Edelsohn
2015-04-27 15:21                   ` Peter Bergner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).