[PATCH,rs6000] Add support for Power9 DFP Test Significance Immediate instruction

Segher Boessenkool segher@kernel.crashing.org
Mon Jun 27 21:35:00 GMT 2016


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



More information about the Gcc-patches mailing list