Re: [PATCH,rs6000] Add built-in function support Power9 binary floating point operations

[Pat Haugen]

On 08/05/2016 08:04 PM, Segher Boessenkool wrote:
> On Fri, Aug 05, 2016 at 04:27:36PM -0500, Pat Haugen wrote:
>> On 08/02/2016 03:15 PM, Segher Boessenkool wrote:
>>> On Tue, Aug 02, 2016 at 03:03:42PM -0500, Pat Haugen wrote:
 On 07/29/2016 10:47 AM, Kelvin Nilsen wrote:
> +  "xsxexpdp %0,%x1"
> +  [(set_attr "type" "fp")])

 Type should be 'integer'.
>>>
>>> It has VSX regs as input, integer is worse than fpsimple here, I think?
>>> Or vecsimple, that seems better yes.
>>
>> It's a 2 cycle op that executes in the ALU pipe, just like other integer 
>> ops. vecsimple would be worse since those consume a superslice, which this 
>> op doesn't.
>>
> 
> Ah yes.  But why not fpsimple?  It schedules the same as e.g. fmr and
> fabs?
> 

The subtle difference is that since it has a GPR target it does not restrict a 
tuple (3 insns) from being dispatched to a superslice, whereas fpsimple ops do.

-Pat

Re: [PATCH,rs6000] Add built-in function support Power9 binary floating point operations

[Segher Boessenkool]

On Fri, Aug 05, 2016 at 04:27:36PM -0500, Pat Haugen wrote:
> On 08/02/2016 03:15 PM, Segher Boessenkool wrote:
> > On Tue, Aug 02, 2016 at 03:03:42PM -0500, Pat Haugen wrote:
> >> On 07/29/2016 10:47 AM, Kelvin Nilsen wrote:
> >>> +  "xsxexpdp %0,%x1"
> >>> +  [(set_attr "type" "fp")])
> >>
> >> Type should be 'integer'.
> > 
> > It has VSX regs as input, integer is worse than fpsimple here, I think?
> > Or vecsimple, that seems better yes.
> 
> It's a 2 cycle op that executes in the ALU pipe, just like other integer ops. 
> vecsimple would be worse since those consume a superslice, which this op 
> doesn't.
> 

Ah yes.  But why not fpsimple?  It schedules the same as e.g. fmr and
fabs?


Segher

Re: [PATCH,rs6000] Add built-in function support Power9 binary floating point operations

[Pat Haugen]

On 08/02/2016 03:15 PM, Segher Boessenkool wrote:
> On Tue, Aug 02, 2016 at 03:03:42PM -0500, Pat Haugen wrote:
>> On 07/29/2016 10:47 AM, Kelvin Nilsen wrote:
>>> +  "xsxexpdp %0,%x1"
>>> +  [(set_attr "type" "fp")])
>>
>> Type should be 'integer'.
> 
> It has VSX regs as input, integer is worse than fpsimple here, I think?
> Or vecsimple, that seems better yes.

It's a 2 cycle op that executes in the ALU pipe, just like other integer ops. 
vecsimple would be worse since those consume a superslice, which this op 
doesn't.

Re: [PATCH,rs6000] Add built-in function support Power9 binary floating point operations

[Segher Boessenkool]

On Tue, Aug 02, 2016 at 03:03:42PM -0500, Pat Haugen wrote:
> On 07/29/2016 10:47 AM, Kelvin Nilsen wrote:
> > +  "xsxexpdp %0,%x1"
> > +  [(set_attr "type" "fp")])
> 
> Type should be 'integer'.

It has VSX regs as input, integer is worse than fpsimple here, I think?
Or vecsimple, that seems better yes.


Segher

Re: [PATCH,rs6000] Add built-in function support Power9 binary floating point operations

[Pat Haugen]

On 07/29/2016 10:47 AM, Kelvin Nilsen wrote:
> +  "xsxexpdp %0,%x1"
> +  [(set_attr "type" "fp")])

Type should be 'integer'.

> +  "xsxsigdp %0,%x1"
> +  [(set_attr "type" "fp")])

'integer'

> +  "xsiexpdp %x0,%1,%2"
> +  [(set_attr "type" "fp")])

'fpsimple'

> +  "xscmpexpdp %0,%x1,%x2"
> +  [(set_attr "type" "fp")])

'fpcompare'

> +  "xststdc %0,%x1,%2"
> +  [(set_attr "type" "fp")])

'fpcompare'

> +  "xvxexp %x0,%x1"
> +  [(set_attr "type" "fp")])

'vecsimple'

> +  "xvxsig %x0,%x1"
> +  [(set_attr "type" "fp")])

'vecsimple'

> +  "xviexp %x0,%x1,%x2"
> +  [(set_attr "type" "fp")])

'vecsimple'

> +  "xvtstdc %x0,%x1,%2"
> +  [(set_attr "type" "fp")])

'vecsimple'

-Pat

Re: [PATCH,rs6000] Add built-in function support Power9 binary floating point operations

[Segher Boessenkool]

Hi Kelvin,

On Fri, Jul 29, 2016 at 09:47:55AM -0600, Kelvin Nilsen wrote:
> This patch adds built-in support for the following fourteen new binary
> floating point instructions introduced with the Power9 architecture: 

Some comments, mostly about whitespace:

> --- gcc/doc/extend.texi   (revision 238014)
> +++ gcc/doc/extend.texi   (working copy)

> +int scalar_test_data_class (float source, unsigned int condition);
> +int scalar_test_data_class (double source, unsigned int condition);
> +
> +int scalar_test_neg (float source)
> +int scalar_test_neg (double source)

These last two probably want a semicolon as well?

> +The @code{scalar_extract_sig} and @code{scalar_insert_exp}
> +functions require a 64-bit environment supporting ISA 3.0 or later.
> +The @code{scalar_extract_exp} and @code{vec_extract_sig} built-in
> +functions return the significand and exponent respectively of their
> +@code{source} arguments.  The 

Trailing space.

> +The @code{scalar_cmp_exp_gt}, @code{scalar_cmp_exp_lt},
> +@code{scalar_cmp_exp_eq}, and @code{scalar_cmp_exp_unordered} built-in
> +functions return a non-zero value if @code{arg1} is greater than, less
> +than, equal to, or not comparable to @code{arg2} respectively.  The
> +arguments are not comparable if one or the other equals NaN (not a
> +number). 

Trailing space.

> +__vector float
> +vec_insert_exp (__vector unsigned int significands,  __vector unsigned int 
> exponents);
> +__vector double
> +vec_insert_exp (__vector unsigned long long int significands,
> +__vector unsigned long long int exponents);

Break up the first of these to two lines as well?

> +/* { dg-skip-if "" { powerpc*-*-aix* } } */

I think you can do this in bfp.exp, for all tests at the same time?
Or will bfp/ include tests that can run on AIX, eventually?

> +/* This test should succeed only 64-bit configuration.  */

Maybe "only on 64-bit configurations"?

> +;; Iterator for scalar floating point types
> +(define_mode_iterator VSX_SF [DF SF])

There is an iterator SFDF already; is there a reason to use a new one?

> +(define_mode_attr vsx_sf_suffix [(DF "dp") (SF "sp")])

That is Fvsx already.

> +(define_mode_attr vsx_f_suffix [(V4SF "dp") (V2DF "sp")])

Those seem swapped?

You can make vsx_sf_suffix (or Fvsx) do these types as well.

> +(define_mode_attr VSX_F_INTEGER [(V4SF "V4SI") (V2DF "V2DI")])

VSi does this, too.

> +;; ISA 3.0 Binary Floating-Point Support
> +
> +;; VSX Scalar Extract Exponent Double-Precision
> +(define_insn "xsxexpdp"
> +  [(set (match_operand:DI 0 "register_operand" "=r")
> + (unspec:DI [(match_operand:DF 1 "vsx_register_operand" "wa")] 
> +UNSPEC_VSX_SXEXPDP))]

These last two lines aren't indented correctly (no spaces before tabs,
and no spaces where you could use a tab).  Many variations below.
"contrib/check_GNU_style.sh" might help a bit.

> +  "xsxexpdp %0,%x1"
> +  [(set_attr "type" "fp")])

I think this should be "fpsimple"?

> +(define_insn "*xscmpexpdp"
> +  [(set (match_operand:CCFP 0 "" "=y")

No predicate?  cc_reg_operand?

> +;; VSX Scalar Test Data Class Double- and Single-Precision
> +;;  (The lt bit is set if operand 1 is negative.  The eq bit is set 
> +;;   if any of the conditions tested by operand 2 are satisfied.  
> +;;   The gt and unordered bits are cleared to zero.)

Trailing spaces.  There are more.

> +(define_expand "xststdc"
> +  [(set (match_dup 3)
> +(compare:CCFP
> + (unspec:VSX_SF
> +  [(match_operand:VSX_SF 1 "vsx_register_operand" "wa")
> +   (match_operand:SI 2 "u7bit_cint_operand" "n")]
> +  UNSPEC_VSX_STSTDC)
> +(match_dup 4)))
> +   (set (match_operand:SI 0 "register_operand" "=r")
> +(eq:SI (match_dup 3)
> +   (const_int 0)))
> +  ]

Don't put this ] on a separate line please.

> +(define_insn "*xststdc"
> +  [(set (match_operand:CCFP 0 "" "=y")
> +(compare:CCFP
> + (unspec:VSX_SF [(match_operand:VSX_SF 1 "vsx_register_operand" "wa")
> +(match_operand:SI 2 "u7bit_cint_operand" "n")]

The ( should line up here.  And, tabs.

> +;; VSX Vector Test Data Class Double and Single Precision
> +;;  The corresponding elements of the result vector are all ones
> +;;   if any of the conditions tested by operand 3 are satisfied.  

Something went wrong with the leading spaces here :-)

> @@ -4721,6 +4799,7 @@ altivec_resolve_overloaded_builtin (location_t loc
>tree types[3], args[3];
>const struct altivec_builtin_types *desc;
>unsigned int n;
> +  bool unsupported_builtin;
>  
>if (!rs6000_overloaded_builtin_p (fcode))
>  return NULL_TREE;
> @@ -5480,6 +5559,7 @@ assignment for unaligned loads and stores");
>return build_int_cst (NULL_TREE, TYPE_VECTOR_SUBPARTS (types[0]));
>  }
>  
> +  unsupported_builtin = false;

Declare it here, instead of 750 lines above?

> +BU_P9V_VSX_2 (VSCEDPGT,  "scalar_cmp_exp_dp_gt", CONST, xscmpexpdp_gt)
> +BU_P9V_VSX_2 

[PATCH,rs6000] Add built-in function support Power9 binary floating point operations

[Kelvin Nilsen]

This patch adds built-in support for the following fourteen new binary
floating point instructions introduced with the Power9 architecture: 

VSX Scalar Extract Exponent Double-Precision
VSX Scalar Extract Significand Double-Precision
VSX Scalar Insert Exponent Double-Precision
VSX Scalar Compare Exponents Double-Precision
VSX Scalar Test Data Class Double-Precision
VSX Scalar Test Data Class Single-Precision
VSX Vector Extract Exponent Double-Precision
VSX Vector Extract Exponent Single-Precision
VSX Vector Extract Significand Double-Precision
VSX Vector Extract Significand Single-Precision
VSX Vector Insert Exponent Double-Precision
VSX Vector Insert Exponent Single-Precision
VSX Vector Test Data Class Double-Precision
VSX Vector Test Data Class Single-Precision

The patch has been bootstrapped and tested on powerpc64le-unknown-linux
and on powerpc64-unknown-linux (big-endian) with no regressions.  Is
this ok for the trunk?

gcc/testsuite/ChangeLog:

2016-07-28  Kelvin Nilsen  

* gcc.target/powerpc/bfp/bfp.exp: New file.
* gcc.target/powerpc/bfp/scalar-cmp-exp-eq-0.c: New test.
* gcc.target/powerpc/bfp/scalar-cmp-exp-eq-1.c: New test.
* gcc.target/powerpc/bfp/scalar-cmp-exp-eq-2.c: New test.
* gcc.target/powerpc/bfp/scalar-cmp-exp-gt-0.c: New test.
* gcc.target/powerpc/bfp/scalar-cmp-exp-gt-1.c: New test.
* gcc.target/powerpc/bfp/scalar-cmp-exp-gt-2.c: New test.
* gcc.target/powerpc/bfp/scalar-cmp-exp-lt-0.c: New test.
* gcc.target/powerpc/bfp/scalar-cmp-exp-lt-1.c: New test.
* gcc.target/powerpc/bfp/scalar-cmp-exp-lt-2.c: New test.
* gcc.target/powerpc/bfp/scalar-cmp-exp-unordered-0.c: New test.
* gcc.target/powerpc/bfp/scalar-cmp-exp-unordered-1.c: New test.
* gcc.target/powerpc/bfp/scalar-cmp-exp-unordered-2.c: New test.
* gcc.target/powerpc/bfp/scalar-extract-exp-0.c: New test.
* gcc.target/powerpc/bfp/scalar-extract-exp-1.c: New test.
* gcc.target/powerpc/bfp/scalar-extract-exp-2.c: New test.
* gcc.target/powerpc/bfp/scalar-extract-sig-0.c: New test.
* gcc.target/powerpc/bfp/scalar-extract-sig-1.c: New test.
* gcc.target/powerpc/bfp/scalar-extract-sig-2.c: New test.
* gcc.target/powerpc/bfp/scalar-insert-exp-0.c: New test.
* gcc.target/powerpc/bfp/scalar-insert-exp-1.c: New test.
* gcc.target/powerpc/bfp/scalar-insert-exp-2.c: New test.
* gcc.target/powerpc/bfp/scalar-test-data-class-0.c: New test.
* gcc.target/powerpc/bfp/scalar-test-data-class-1.c: New test.
* gcc.target/powerpc/bfp/scalar-test-data-class-2.c: New test.
* gcc.target/powerpc/bfp/scalar-test-data-class-3.c: New test.
* gcc.target/powerpc/bfp/scalar-test-data-class-4.c: New test.
* gcc.target/powerpc/bfp/scalar-test-data-class-5.c: New test.
* gcc.target/powerpc/bfp/scalar-test-data-class-6.c: New test.
* gcc.target/powerpc/bfp/scalar-test-data-class-7.c: New test.
* gcc.target/powerpc/bfp/scalar-test-neg-0.c: New test.
* gcc.target/powerpc/bfp/scalar-test-neg-1.c: New test.
* gcc.target/powerpc/bfp/scalar-test-neg-2.c: New test.
* gcc.target/powerpc/bfp/scalar-test-neg-3.c: New test.
* gcc.target/powerpc/bfp/vec-extract-exp-0.c: New test.
* gcc.target/powerpc/bfp/vec-extract-exp-1.c: New test.
* gcc.target/powerpc/bfp/vec-extract-exp-2.c: New test.
* gcc.target/powerpc/bfp/vec-extract-exp-3.c: New test.
* gcc.target/powerpc/bfp/vec-extract-sig-0.c: New test.
* gcc.target/powerpc/bfp/vec-extract-sig-1.c: New test.
* gcc.target/powerpc/bfp/vec-extract-sig-2.c: New test.
* gcc.target/powerpc/bfp/vec-extract-sig-3.c: New test.
* gcc.target/powerpc/bfp/vec-insert-exp-0.c: New test.
* gcc.target/powerpc/bfp/vec-insert-exp-1.c: New test.
* gcc.target/powerpc/bfp/vec-insert-exp-2.c: New test.
* gcc.target/powerpc/bfp/vec-insert-exp-3.c: New test.
* gcc.target/powerpc/bfp/vec-test-data-class-0.c: New test.
* gcc.target/powerpc/bfp/vec-test-data-class-1.c: New test.
* gcc.target/powerpc/bfp/vec-test-data-class-2.c: New test.
* gcc.target/powerpc/bfp/vec-test-data-class-3.c: New test.
* gcc.target/powerpc/bfp/vec-test-data-class-4.c: New test.
* gcc.target/powerpc/bfp/vec-test-data-class-5.c: New test.
* gcc.target/powerpc/bfp/vec-test-data-class-6.c: New test.
* gcc.target/powerpc/bfp/vec-test-data-class-7.c: New test.

gcc/ChangeLog:

2016-07-28  Kelvin Nilsen  

* doc/extend.texi (PowerPC AltiVec Built-in Functions): Document
built-in functions to represent the Power9 binary floating-point
support instructions.
* config/rs6000/rs6000-c.c (altivec_overloaded_builtins): Add
overloaded binary floating point functions.