On Mon, Jul 12, 2021 at 1:47 PM Bill Schmidt <wschm...@linux.ibm.com> wrote:
>
> On 7/12/21 12:16 PM, Michael Meissner wrote:
> > On Sun, Jul 11, 2021 at 02:55:04PM -0500, Bill Schmidt wrote:
> >> Hi Mike,
> >>
> >> On 7/7/21 3:04 PM, Michael Meissner wrote:
> >>> [PATCH] PR 100167: Fix vector long long multiply/divide tests on power10.
> >>>
> >>> This patch updates the vector long long multiply and divide tests to
> >>> supply the correct code information if power10 code generation is used.
> >>>
> >>> 2021-07-07 Michael Meissner <meiss...@linux.ibm.com>
> >>>
> >>> gcc/testsuite/
> >>> PR testsuite/100167
> >>> * gcc.target/powerpc/fold-vec-div-longlong.c:
> >> Missing information after colon.
> > Because all of the changes were the same thing, and the line is long
> > enough. I
> > just grouped all of the files together, and put the change line as the last
> > entry.
> But that's not accepted style. Put it after the first one and use
> "Likewise" is the usual thing. This looks like an omission.
Mike, that's a very creative idea, but let's stick to the precedent of
using Likewise or Same for repeated changes.
> >
> >>> * gcc.target/powerpc/fold-vec-mult-longlong.c: Fix expected code
> >>> generation on power10.
> >>> ---
> >>> gcc/testsuite/gcc.target/powerpc/fold-vec-div-longlong.c | 7 +++++--
> >>> gcc/testsuite/gcc.target/powerpc/fold-vec-mult-longlong.c | 3 ++-
> >>> 2 files changed, 7 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-div-longlong.c
> >>> b/gcc/testsuite/gcc.target/powerpc/fold-vec-div-longlong.c
> >>> index 312e984d3cc..f6a9b290ae5 100644
> >>> --- a/gcc/testsuite/gcc.target/powerpc/fold-vec-div-longlong.c
> >>> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-div-longlong.c
> >>> @@ -19,5 +19,8 @@ test6 (vector unsigned long long x, vector unsigned
> >>> long long y)
> >>> {
> >>> return vec_div (x, y);
> >>> }
> >>> -/* { dg-final { scan-assembler-times {\mdivd\M} 2 } } */
> >>> -/* { dg-final { scan-assembler-times {\mdivdu\M} 2 } } */
> >>> +
> >>> +/* { dg-final { scan-assembler-times {\mdivd\M} 2 { target { !
> >>> has_arch_pwr10 } } } } */
> >>> +/* { dg-final { scan-assembler-times {\mdivdu\M} 2 { target { !
> >>> has_arch_pwr10 } } } } */
> >>> +/* { dg-final { scan-assembler-times {\mvdivsd\M} 1 { target {
> >>> has_arch_pwr10 } } } } */
> >>> +/* { dg-final { scan-assembler-times {\mvdivud\M} 1 { target {
> >>> has_arch_pwr10 } } } } */
> >>> diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-mult-longlong.c
> >>> b/gcc/testsuite/gcc.target/powerpc/fold-vec-mult-longlong.c
> >>> index 38dba9f5023..bd210e34801 100644
> >>> --- a/gcc/testsuite/gcc.target/powerpc/fold-vec-mult-longlong.c
> >>> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mult-longlong.c
> >>> @@ -20,5 +20,6 @@ test6 (vector unsigned long long x, vector unsigned
> >>> long long y)
> >>> return vec_mul (x, y);
> >>> }
> >>> -/* { dg-final { scan-assembler-times "\[ \t\]mulld " 4 { target lp64 } }
> >>> } */
> >>> +/* { dg-final { scan-assembler-times {\mmulld\M} 4 { target { lp64 && {
> >>> ! has_arch_pwr10 } } } } } */
> >>> +/* { dg-final { scan-assembler-times {\mvmulld\M} 2 { target {
> >>> has_arch_pwr10 } } } } */
> >> Shouldn't this last be { lp64 && has_arch_pwr10 } ?
> > Nope. Because the power10 vector multiply is done in the vector unit, it
> > can
> > generate the vmulld instruction.
>
> Please document this, then.
>
> Thanks,
> Bill
>
> >
> >> Otherwise LGTM. I can't approve, but recommend approval with those
> >> changes.
Okay with a correct ChangeLog and a comment on the vmulld line.
Thanks, David
