[PATCH] rs6000, vector integer multiply/divide/modulo instructions

Segher Boessenkool segher@kernel.crashing.org
Thu Nov 19 23:26:52 GMT 2020


On Wed, Nov 04, 2020 at 08:44:03AM -0800, Carl Love wrote:
> +#define vec_mulh(a, b) __builtin_vec_mulh (a, b)
> +#define vec_div(a, b) __builtin_vec_div (a, b)
> +#define vec_dive(a, b) __builtin_vec_dive (a, b)
> +#define vec_mod(a, b) __builtin_vec_mod (a, b)

This should be

#define vec_mulh(a, b) __builtin_vec_mulh ((a), (b))

etc...  I see we have quite a few cases in altivec.h already that do not
get that right.  Something to fix, and apparently not too important in
practice ;-)

>  ;; Short vec int modes
>  (define_mode_iterator VIshort [V8HI V16QI])
> -;; Longer vec int modes for rotate/mask ops
> -(define_mode_iterator VIlong [V2DI V4SI])

Hrm, you move this one to vsx.md, but leave VIshort here (instead of
moving that to altivec.md).  Oh well, something needs to be done about
this split anyway.

> +BU_P10V_AV_2 (VDIVES_V4SI, "vdivesw", CONST, vdives_v4si)
> +BU_P10V_AV_2 (VDIVES_V2DI, "vdivesd", CONST, vdives_v2di)
> +BU_P10V_AV_2 (VDIVEU_V4SI, "vdiveuw", CONST, vdiveu_v4si)
> +BU_P10V_AV_2 (VDIVEU_V2DI, "vdiveud", CONST, vdiveu_v2di)
> +BU_P10V_AV_2 (VDIVS_V4SI, "vdivsw", CONST, divv4si3)
> +BU_P10V_AV_2 (VDIVS_V2DI, "vdivsd", CONST, divv2di3)
> +BU_P10V_AV_2 (VDIVU_V4SI, "vdivuw", CONST, udivv4si3)
> +BU_P10V_AV_2 (VDIVU_V2DI, "vdivud", CONST, udivv2di3)
> +BU_P10V_AV_2 (VMODS_V2DI, "vmodsd", CONST, vmods_v2di)
> +BU_P10V_AV_2 (VMODS_V4SI, "vmodsw", CONST, vmods_v4si)
> +BU_P10V_AV_2 (VMODU_V2DI, "vmodud", CONST, vmodu_v2di)
> +BU_P10V_AV_2 (VMODU_V4SI, "vmoduw", CONST, vmodu_v4si)
> +BU_P10V_AV_2 (VMULHS_V2DI, "vmulhsd", CONST, vmulhs_v2di)
> +BU_P10V_AV_2 (VMULHS_V4SI, "vmulhsw", CONST, vmulhs_v4si)
> +BU_P10V_AV_2 (VMULHU_V2DI, "vmulhud", CONST, vmulhu_v2di)
> +BU_P10V_AV_2 (VMULHU_V4SI, "vmulhuw", CONST, vmulhu_v4si)
> +BU_P10V_AV_2 (VMULLD_V2DI, "vmulld", CONST, mulv2di3)

So I would remove the leading "v" from all these pattern names, since
all of them have a mode in the name already.

> +(define_mode_attr VIlong_char [(V2DI "d")
> +			       (V4SI "w")])

This is just a subset of <wd> -- use that, instead?

; A generic w/d attribute, for things like cmpw/cmpd.
(define_mode_attr wd [(QI    "b")
                      (HI    "h")
                      (SI    "w")
                      (DI    "d")
                      (V16QI "b")
                      (V8HI  "h")
                      (V4SI  "w")
                      (V2DI  "d")
                      (V1TI  "q")
                      (TI    "q")])

(never mind the name, heh -- it still is nice and short ;-) )

> +(define_insn "vmulhs_<mode>"
> +  [(set (match_operand:VIlong 0 "vsx_register_operand" "=v")
> +	(unspec:VIlong [(match_operand:VIlong 1 "vsx_register_operand" "v")
> +		        (match_operand:VIlong 2 "vsx_register_operand" "v")]
> +		       UNSPEC_VMULHS))]
> +  "TARGET_POWER10"
> +  "vmulhs<VIlong_char> %0,%1,%2"
> +  [(set_attr "type" "vecsimple")])

The scalar mulh we can describe without unspecs, cannot that be done
here as well?

The type attr is problematic...  At least make it the same as the other
vector int multiplies?  That is veccomplex?

> +Vector Integer Multiply-Divide-Modulo

Use "/" instead of "-" here?  "-" normally is used for things like
"multiply-sum", not to mean "or".

> +For each integer value i from 0 to 3, do the following. The integer value in
> +word element i of a is multiplied by the integer value in word
> +element i of b. The high-order 32 bits of the 64-bit product are placed into
> +word element i of the vector returned.

I think you should quote the "i"?  @code{i} or similar.  I don't think
you need to mark up the digits, phew :-)

Please repost with those things fixed?  Thanks!


Segher


More information about the Gcc-patches mailing list