diff mbox

[rs6000,testsuite] Fix PR target/64579, __TM_end __builtin_tend failed to return transactional state

Message ID 1426879660.13627.71.camel@otta
State New
Headers show

Commit Message

Peter Bergner March 20, 2015, 7:27 p.m. UTC 
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.

Comments

Segher Boessenkool March 20, 2015, 8:52 p.m. UTC | #1 
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
Peter Bergner March 20, 2015, 10:41 p.m. UTC | #2 
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
diff mbox

Patch

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 }