This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, rs6000, testsuite] Fix PR target/64579, __TM_end __builtin_tend failed to return transactional state
- From: Segher Boessenkool <segher at kernel dot crashing dot org>
- To: Peter Bergner <bergner at vnet dot ibm dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, David Edelsohn <dje dot gcc at gmail dot com>, Michael Meissner <meissner at linux dot vnet dot ibm dot com>, Bill Schmidt <wschmidt at linux dot vnet dot ibm dot com>
- Date: Fri, 20 Mar 2015 15:52:00 -0500
- Subject: Re: [PATCH, rs6000, testsuite] Fix PR target/64579, __TM_end __builtin_tend failed to return transactional state
- Authentication-results: sourceware.org; auth=none
- References: <1426879660 dot 13627 dot 71 dot camel at otta>
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