Re: [PATCH v4, rs6000] gcc mainline, add builtin support for vec_float, vec_float2, vec_floate, vec_floate, builtins

[Segher Boessenkool <segher@kernel.crashing.org>]

Hi Carl,

On Fri, Jun 16, 2017 at 09:23:06AM -0700, Carl Love wrote:
> 	* config/rs6000/rs6000-c.c (altivec_overloaded_builtins[]): Add
> 	definitions for vec_float, vec_float2, vec_floato,
> 	vec_floate built-ins.

No [], just the name.

> 	* config/rs6000/rs6000-builtin.def (FLOAT2_V2DI, FLOATE_V2D*,
> 	FLOATO_V2D*, XVCVSXWSP_V4SF, UNS_FLOATO_V2DI, UNS_FLOATE_V2DI): Add
> 	definitions.

Please spell out FLOATE_V2DF, FLOATE_V2DI -- it's only two of-em, and
it makes things easier to find.

> 	* config/altivec.md (define_insn "p8_vmrgew_<mode>",
> 	define_mode_attr VF_sxddp):Add V4SF type to p8_vmrgew.

Space after colon.

> 	* gcc.target/powerpc/builtins-3-runnable.c (test_result_sp(),
> 	main()): Add runnable tests and test checker for vec_float,
> 	vec_float2, vec_floate and vec_floato builtins.

No () please.

> +(define_insn "vsx_xvcvsxwsp"
> +  [(set (match_operand:V4SF 0 "vsx_register_operand" "=wa")
> +	(unspec:V4SF [(match_operand:V4SI 1 "vsx_register_operand" "wa")]
> +		     UNSPEC_VSX_CVSXWSP))]
> +  "VECTOR_UNIT_VSX_P (V4SFmode)"
> +  "xvcvsxwsp %x0,%x1"
> +  [(set_attr "type" "vecdouble")])

Hrm, is that the best type?  Maybe vecfloat is better.

> +;; Generate floate
> +;; convert  double or long long signed to float
> +;;(Only even words are valid, BE numbering)

Single space before double; space before (.

> +(define_expand "floato<mode>"
> + [(use (match_operand:V4SF 0 "register_operand" "=wa"))
> +  (use (match_operand:VSX_D 1 "register_operand" "wa"))]
> + "VECTOR_UNIT_VSX_P (V4SFmode)"

These last three lines should be indented one more space.

> +{
> +  if (VECTOR_ELT_ORDER_BIG)
> +    emit_insn (gen_vsx_xvcv<VFC_inst>sp (operands[0], operands[1]));
> +  else
> +    {
> +      /* Shift left one word to put odd word correct location */
> +      rtx rtx_tmp;
> +      rtx rtx_val = GEN_INT (4);
> +
> +      rtx_tmp = gen_reg_rtx (V4SFmode);
> +      emit_insn (gen_vsx_xvcv<VFC_inst>sp (rtx_tmp, operands[1]));
> +      emit_insn (gen_altivec_vsldoi_v4sf (operands[0],
> +                 rtx_tmp, rtx_tmp, rtx_val));

This indent should use tabs.  There are more like this.

Okay with those last trivialities fixed.  Thanks!


Segher