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
