Kewen:
On Mon, 2023-07-31 at 14:53 +0800, Kewen.Lin wrote:
> 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]/
OK, updated in email for version 2.
>
> > 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.
OK, replaced Power 8 with Altivec here and for additional instances of
Power 8 below.
>
> > 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"?
Fixed, fixed.
>
> > 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.
fixed
>
> > 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.
I looked at just adding -save-temp and scan-assembler-times for the
instructions. I noticed that vcmpequw occurs 30 times in the functions
to initialize and test the results. So, I opted to create a separate
compile/check instructions test and a runnable test to verify the
functionality. This way any changes in the code to calculate and
verify the results will not break the instruction generation checks.
>
> Coverage for unsigned long long int and long long int
> > for Power 10 in int_128bit-runnable.c.
Removed comment about Power 10, long long int testing.
> >
> > 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.
Removed.
Carl
