Hi Kelvin,
On Mon, Jun 27, 2016 at 11:47:52AM -0600, Kelvin Nilsen wrote:
>
> This patch adds built-in function support for the new Power9 dtstsfi
> instruction.
>
> This has bootstrapped and regression tested on
> powerpc64le-unknown-linux-gnu without regressions. Is this ok for the
> trunk? Is this patch ok for gcc-6 after some burn-in time on the trunk?
Okay for trunk with the fixes below. Okay for 6 later, too.
> * gcc.target/powerpc/dtstsfi-0.c: New test.
..
> * gcc.target/powerpc/dtstsfi-79.c: New test.
If you have this many tests, please use a subdir.
> (rs6000_expand_binop_builtin): Enforce that argument 0 of the exp
> argument is a 6-bit unsigned literal value if the icode argument
Duplicated space.
> +(define_expand "dfptstsfi_<code>_<mode>"
> + [(set (match_dup 3)
> + (compare:CCFP
> + (unspec:D64_D128
Trailing space.
> + [(match_operand:SI 1 "const_int_operand" "n")
> + (match_operand:D64_D128 2 "gpc_reg_operand" "d")]
> + UNSPEC_DTSTSFI)
> + (match_dup 4)))
> + (set (match_operand:SI 0 "register_operand" "")
> + (DFP_TEST:SI (match_dup 3)
Trailing space.
> + (const_int 0)))
> + ]
> + "TARGET_P9_MISC"
> +{
> + operands[3] = gen_reg_rtx (CCFPmode);
> + operands[4] = CONST0_RTX (SImode);
That's just const0_rtx.
> +(define_insn "*dfp_sgnfcnc_<mode>"
> + [(set (match_operand:CCFP 0 "" "=y")
> + (compare:CCFP
> + (unspec:D64_D128 [(match_operand:SI 1 "const_int_operand" "n")
> + (match_operand:D64_D128 2 "gpc_reg_operand" "d")]
> + UNSPEC_DTSTSFI)
> + (match_operand:SI 3 "zero_constant" "j")))]
> + "TARGET_P9_MISC"
> + {
Please left-align the {} block.
> + /* If immediate operand is greater than 63, it will behave as if
> + * the value had been 63. The code generator does not support
> + * immediate operand values greater than 63. */
No leading * on comment continuation lines. Two spaces after full stop.
Trailing space.
> + else if (icode == CODE_FOR_dfptstsfi_eq_dd
> + || icode == CODE_FOR_dfptstsfi_lt_dd
> + || icode == CODE_FOR_dfptstsfi_gt_dd
> + || icode == CODE_FOR_dfptstsfi_unordered_dd
> + || icode == CODE_FOR_dfptstsfi_eq_td
> + || icode == CODE_FOR_dfptstsfi_lt_td
> + || icode == CODE_FOR_dfptstsfi_gt_td
> + || icode == CODE_FOR_dfptstsfi_unordered_td)
Many trailing spaces.
> + {
> + /* Only allow 6-bit unsigned literals. */
> + STRIP_NOPS (arg0);
> + if (TREE_CODE (arg0) != INTEGER_CST
> + || TREE_INT_CST_LOW (arg0) & ~0x3f)
Use IN_RANGE instead? This only tests the bits that fit in a host int.
> +/* Miscellaneous builtins for decimal floating point instructions
> + added in ISA 3.0. These instructions don't require the VSX
> + options, just the basic ISA 3.0 enablement since they operate on
> + general purpose registers. */
Trailing space.
Thanks,
Segher
