Hi Carl,
on 2023/7/28 23:00, Carl Love wrote:
> GCC maintainers:
>
> The following patch cleans up the definition for the
> __builtin_altivec_vcmpnet. The current implementation implies that the
s/__builtin_altivec_vcmpnet/__builtin_altivec_vcmpne[bhw]/
> built-in is only supported on Power 9 since it is defined under the
> Power 9 stanza. However the built-in has no ISA restrictions as stated
> in the Power Vector Intrinsic Programming Reference document. The
> current built-in works because the built-in gets replaced during GIMPLE
> folding by a simple not-equal operator so it doesn't get expanded and
> checked for Power 9 code generation.
>
> This patch moves the definition to the Altivec stanza in the built-in
> definition file to make it clear the built-ins are valid for Power 8,
> Power 9 and beyond.
>
> The patch has been tested on Power 8 LE/BE, Power 9 LE/BE and Power 10
> LE with no regressions.
>
> Please let me know if the patch is acceptable for mainline. Thanks.
>
> Carl
>
> --------------------------------------
> rs6000: Fix __builtin_altivec_vcmpne{b,h,w} implementation
>
> The current built-in definitions for vcmpneb, vcmpneh, vcmpnew are defined
> under the Power 9 section of r66000-builtins. This implies they are only
> supported on Power 9 and above when in fact they are defined and work on
> Power 8 as well with the appropriate Power 8 instruction generation.
Nit: It's confusing to say Power8 only, it's actually supported once altivec
is enabled, so I think it's more clear to replace Power8 with altivec here.
>
> The vec_cmpne builtin should generate the vcmpequ{b,h,w} instruction on
> Power 8 and generate the vcmpne{b,h,w} on Power 9 an newer processors.
Ditto for Power8 and "an" -> "and"?
>
> This patch moves the definitions to the Altivec stanza to make it clear
> the built-ins are supported for all Altivec processors. The patch
> enables the vcmpequ{b,h,w} instruction to be generated on Power 8 and
> the vcmpne{b,h,w} instruction to be generated on Power 9 and beyond.
Ditto for Power8.
>
> There is existing test coverage for the vec_cmpne built-in for
> vector bool char, vector bool short, vector bool int,
> vector bool long long in builtins-3-p9.c and p8vector-builtin-2.c.
> Coverage for vector signed int, vector unsigned int is in
> p8vector-builtin-2.c.
So there is no coverage with the basic altivec support. I noticed
we have one test case "gcc/testsuite/gcc.target/powerpc/vec-cmpne.c"
which is a test case for running but with vsx_ok, I think we can
rewrite it with altivec (vmx), either separating to compiling and
running case, or adding -save-temp and check expected insns.
Coverage for unsigned long long int and long long int
> for Power 10 in int_128bit-runnable.c.
>
> Patch has been tested on Power 8 LE/BE, Power 9 LE/BE and Power 10 LE
> with no regressions.
>
> gcc/ChangeLog:
>
> * config/rs6000/rs6000-builtins.def (vcmpneb, vcmpneh, vcmpnew.
> vcmpnet): Move definitions to Altivec stanza.
vcmpnet which isn't handled in this patch should be removed.
The others look good to me, thanks!
BR,
Kewen
