[Bug target/108396] New: PPCLE: vec_vsubcuq missing

[Bug target/108396] New: PPCLE: vec_vsubcuq missing

[jens.seifert at de dot ibm.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108396

            Bug ID: 108396
           Summary: PPCLE: vec_vsubcuq missing
           Product: gcc
           Version: 12.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jens.seifert at de dot ibm.com
  Target Milestone: ---

Input:

#include <altivec.h>

vector unsigned __int128 vsubcuq(vector unsigned __int128 a, vector unsigned
__int128 b)
{
    return vec_vsubcuq(a, b);
}

Command line:
gcc -m64 -O2 -maltivec -mcpu=power8 text.C

Output:
<source>: In function '__vector unsigned __int128 vsubcuq(__vector unsigned
__int128, __vector unsigned __int128)':
<source>:6:12: error: 'vec_vsubcuq' was not declared in this scope; did you
mean 'vec_vsubcuqP'?
    6 |     return vec_vsubcuq(a, b);
      |            ^~~~~~~~~~~
      |            vec_vsubcuqP
Compiler returned: 1

[Bug target/108396] [12/13 Regression] PPCLE: vec_vsubcuq missing

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108396

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |rejects-valid
            Summary|PPCLE: vec_vsubcuq missing  |[12/13 Regression] PPCLE:
                   |                            |vec_vsubcuq missing
   Last reconfirmed|                            |2023-01-13
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Target Milestone|---                         |12.3

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
This looks like a regression introduced by r12-3169-ge2693a72701fe5 .
rs6000-overload.def:[VEC_VSUBCUQ, vec_vsubcuqP, __builtin_vec_vsubcuq]


There looks like a typo of including a P in the overload name for some reason
...

[Bug target/108396] [12/13 Regression] PPCLE: vec_vsubcuq missing

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108396

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug target/108396] [12/13 Regression] PPCLE: vec_vsubcuq missing since r12-5752-gd08236359eb229

[linkw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108396

Kewen Lin <linkw at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
            Summary|[12/13 Regression] PPCLE:   |[12/13 Regression] PPCLE:
                   |vec_vsubcuq missing         |vec_vsubcuq missing since
                   |                            |r12-5752-gd08236359eb229
           Assignee|unassigned at gcc dot gnu.org      |linkw at gcc dot gnu.org
                 CC|                            |linkw at gcc dot gnu.org

--- Comment #2 from Kewen Lin <linkw at gcc dot gnu.org> ---
Yes, it's a typo, which makes the macro definition change to:

#define vec_vsubcuqP __builtin_vec_vsubcuq

Unfortunately we don't have the testing coverage in testsuite for the expected
name vec_vsubcuq (in rs6000-vecdefines.h):

$grep -ir vsubcuq gcc/testsuite/gcc.target/powerpc/
gcc/testsuite/gcc.target/powerpc//p8vector-int128-1.c:  return
__builtin_vec_vsubcuq (p, q);
gcc/testsuite/gcc.target/powerpc//p8vector-int128-1.c:/* { dg-final {
scan-assembler       "vsubcuq"   } } */
gcc/testsuite/gcc.target/powerpc//p8vector-builtin-8.c:/* { dg-final {
scan-assembler-times "vsubcuq" 2 } } */

FWIW, it started to fail from r12-5752-gd08236359eb229 which enables the new
bif framework, though the commit r12-3169 introduced it as Andrew pointed out.

[Bug target/108396] [12/13 Regression] PPCLE: vec_vsubcuq missing since r12-5752-gd08236359eb229

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108396

--- Comment #3 from Segher Boessenkool <segher at gcc dot gnu.org> ---
(In reply to Kewen Lin from comment #2)
> Yes, it's a typo, which makes the macro definition change to:
> 
> #define vec_vsubcuqP __builtin_vec_vsubcuq

Yup.

> Unfortunately we don't have the testing coverage in testsuite for the
> expected name vec_vsubcuq (in rs6000-vecdefines.h):

Is it hard to add one for all (or many) of the legacy builtins?  Do we want
to test more than just if it compiles?

[Bug target/108396] [12/13 Regression] PPCLE: vec_vsubcuq missing since r12-5752-gd08236359eb229

[linkw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108396

--- Comment #4 from Kewen Lin <linkw at gcc dot gnu.org> ---
(In reply to Segher Boessenkool from comment #3)
> (In reply to Kewen Lin from comment #2)
> > Unfortunately we don't have the testing coverage in testsuite for the
> > expected name vec_vsubcuq (in rs6000-vecdefines.h):
> 
> Is it hard to add one for all (or many) of the legacy builtins?  Do we want
> to test more than just if it compiles?

Good question, I think it's not hard to add several (classified as their
stanzas) for them and testing the compilation is enough. I just filed one
internal issue for tracking it in GCC14, it's to visit all bif names defined in
header file and get or create one in test suite.

[Bug target/108396] [12/13 Regression] PPCLE: vec_vsubcuq missing since r12-5752-gd08236359eb229

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108396

--- Comment #5 from Segher Boessenkool <segher at gcc dot gnu.org> ---
(In reply to Segher Boessenkool from comment #3)
> Is it hard to add one for all (or many) of the legacy builtins?  Do we want
> to test more than just if it compiles?

Btw, "legacy"...  I thought (from the names) this is one of the old VMX
things, but it is the "q" version, ISA 2.07 .  But it still would be nice
if we could at least have trivial compilation tests for this :-)

[Bug target/108396] [12/13 Regression] PPCLE: vec_vsubcuq missing since r12-5752-gd08236359eb229

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108396

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Kewen Lin <linkw@gcc.gnu.org>:

https://gcc.gnu.org/g:aaf29ae6cdbaad58b709a77784375d15138174b3

commit r13-5237-gaaf29ae6cdbaad58b709a77784375d15138174b3
Author: Kewen Lin <linkw@linux.ibm.com>
Date:   Wed Jan 18 02:34:25 2023 -0600

    rs6000: Fix typo on vec_vsubcuq in rs6000-overload.def [PR108396]

    As Andrew pointed out in PR108396, there is one typo in
    rs6000-overload.def on built-in function vec_vsubcuq:

      [VEC_VSUBCUQ, vec_vsubcuqP, __builtin_vec_vsubcuq]

    "vec_vsubcuqP" should be "vec_vsubcuq", this typo caused
    us to define vec_vsubcuqP in rs6000-vecdefines.h instead
    of vec_vsubcuq, so that compiler is not able to realize
    the built-in function name vec_vsubcuq any more.

    Co-authored-By: Andrew Pinski <apinski@marvell.com>

            PR target/108396

    gcc/ChangeLog:

            * config/rs6000/rs6000-overload.def (VEC_VSUBCUQ): Fix typo
            vec_vsubcuqP with vec_vsubcuq.

    gcc/testsuite/ChangeLog:

            * gcc.target/powerpc/pr108396.c: New test.

[Bug target/108396] [12/13 Regression] PPCLE: vec_vsubcuq missing since r12-5752-gd08236359eb229

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108396

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Kewen Lin <linkw@gcc.gnu.org>:

https://gcc.gnu.org/g:cb6861acc4074fd2c30a96b52d68c2cd33b9e94d

commit r12-9170-gcb6861acc4074fd2c30a96b52d68c2cd33b9e94d
Author: Kewen Lin <linkw@linux.ibm.com>
Date:   Wed Jan 18 02:34:25 2023 -0600

    rs6000: Fix typo on vec_vsubcuq in rs6000-overload.def [PR108396]

    As Andrew pointed out in PR108396, there is one typo in
    rs6000-overload.def on built-in function vec_vsubcuq:

      [VEC_VSUBCUQ, vec_vsubcuqP, __builtin_vec_vsubcuq]

    "vec_vsubcuqP" should be "vec_vsubcuq", this typo caused
    us to define vec_vsubcuqP in rs6000-vecdefines.h instead
    of vec_vsubcuq, so that compiler is not able to realize
    the built-in function name vec_vsubcuq any more.

    Co-authored-By: Andrew Pinski <apinski@marvell.com>

            PR target/108396

    gcc/ChangeLog:

            * config/rs6000/rs6000-overload.def (VEC_VSUBCUQ): Fix typo
            vec_vsubcuqP with vec_vsubcuq.

    gcc/testsuite/ChangeLog:

            * gcc.target/powerpc/pr108396.c: New test.

    (cherry picked from commit aaf29ae6cdbaad58b709a77784375d15138174b3)

[Bug target/108396] [12/13 Regression] PPCLE: vec_vsubcuq missing since r12-5752-gd08236359eb229

[linkw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108396

Kewen Lin <linkw at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #8 from Kewen Lin <linkw at gcc dot gnu.org> ---
Fixed on trunk.

[Bug target/108396] [12/13 Regression] PPCLE: vec_vsubcuq missing since r12-5752-gd08236359eb229

[carll at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108396

Carl Love <carll at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |carll at gcc dot gnu.org

--- Comment #9 from Carl Love <carll at gcc dot gnu.org> ---
I made a copy of rs6000-overload.def and then with a series of emacs macros
converted the list of builtins to a script to grep for the builtins in the test
directory.  Specifically from rs6000-overload.def:

[BCDADD, __builtin_bcdadd, __builtin_vec_bcdadd]
  vsq __builtin_vec_bcdadd (vsq, vsq, const int);
    BCDADD_V1TI
  vuc __builtin_vec_bcdadd (vuc, vuc, const int);
    BCDADD_V16QI

[BCDADD_EQ, __builtin_bcdadd_eq, __builtin_vec_bcdadd_eq]
  signed int __builtin_vec_bcdadd_eq (vsq, vsq, const int);
    BCDADD_EQ_V1TI
  signed int __builtin_vec_bcdadd_eq (vuc, vuc, const int);
    BCDADD_EQ_V16QI

....

Was converted to the bash script:

rm -f ../test1_not_found 

NOT_FOUND='0       0       0'
check_name () {
  str1=$(grep -r  $1 * | wc)

#  echo " output of command: $str1"

  if [[ "$str1" == *"$NOT_FOUND"* ]]; then
    echo "$1 not found" >> ../test1_not_found
  fi
}

check_name "__builtin_bcdadd" "__builtin_vec_bcdadd"

check_name "__builtin_bcdadd_eq" "__builtin_vec_bcdadd_eq"

....

The script is passed the user built-in name ($str1) and the internal built-in
name ($str2).  I ran the script in directory gcc/testsuite/gcc.target/powerpc
and it identified two tests ($str1) as not showing up in a test file.  The
tests were:  __builtin_bcdsub_ge and __builtin_bcdsub_le.

I figure if the first builtin name has a test associated with it that should be
sufficient.  I will create a patch to add testcases for the two missing
builtin-names.

I did add to the script to see how many definitions have a test for the
built-in name $1 but not the built-in name $2 doesn't show up in a test file. 
My script identified 86 of these cases.  Not sure that we really need to add
test cases for the internal builtin name ($str).  Thoughts?

[Bug target/108396] [12/13 Regression] PPCLE: vec_vsubcuq missing since r12-5752-gd08236359eb229

[linkw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108396

--- Comment #10 from Kewen Lin <linkw at gcc dot gnu.org> ---
(In reply to Carl Love from comment #9)
> I made a copy of rs6000-overload.def and then with a series of emacs macros
> converted the list of builtins to a script to grep for the builtins in the
> test directory.  Specifically from rs6000-overload.def:
> 
> [BCDADD, __builtin_bcdadd, __builtin_vec_bcdadd]
>   vsq __builtin_vec_bcdadd (vsq, vsq, const int);
>     BCDADD_V1TI
>   vuc __builtin_vec_bcdadd (vuc, vuc, const int);
>     BCDADD_V16QI
> 
> [BCDADD_EQ, __builtin_bcdadd_eq, __builtin_vec_bcdadd_eq]
>   signed int __builtin_vec_bcdadd_eq (vsq, vsq, const int);
>     BCDADD_EQ_V1TI
>   signed int __builtin_vec_bcdadd_eq (vuc, vuc, const int);
>     BCDADD_EQ_V16QI
> 
> ....
> 
> Was converted to the bash script:
> 
> rm -f ../test1_not_found 
> 
> NOT_FOUND='0       0       0'
> check_name () {
>   str1=$(grep -r  $1 * | wc)
> 
> #  echo " output of command: $str1"
> 
>   if [[ "$str1" == *"$NOT_FOUND"* ]]; then
>     echo "$1 not found"	>> ../test1_not_found
>   fi
> }
> 
> check_name "__builtin_bcdadd" "__builtin_vec_bcdadd"
> 
> check_name "__builtin_bcdadd_eq" "__builtin_vec_bcdadd_eq"
> 
> ....
>  
> The script is passed the user built-in name ($str1) and the internal
> built-in name ($str2).  I ran the script in directory
> gcc/testsuite/gcc.target/powerpc
> and it identified two tests ($str1) as not showing up in a test file.  The
> tests were:  __builtin_bcdsub_ge and __builtin_bcdsub_le.
> 
> I figure if the first builtin name has a test associated with it that should
> be sufficient.  I will create a patch to add testcases for the two missing
> builtin-names.
> 
> I did add to the script to see how many definitions have a test for the
> built-in name $1 but not the built-in name $2 doesn't show up in a test
> file.  My script identified 86 of these cases.  Not sure that we really need
> to add test cases for the internal builtin name ($str).  Thoughts?

Thanks for tracking this!  As you said, I think checking the name $1 is
sufficient since only those are visible for users. btw, since this is closed,
let us discuss it on the internal issue further. :)