[Bug target/110132] New: aarch64: Bogus -Wbuiltin-declaration-mismatch with ls64 builtins

[Bug target/110132] New: aarch64: Bogus -Wbuiltin-declaration-mismatch with ls64 builtins

[acoplan at gcc dot gnu.org]

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

            Bug ID: 110132
           Summary: aarch64: Bogus -Wbuiltin-declaration-mismatch with
                    ls64 builtins
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: acoplan at gcc dot gnu.org
  Target Milestone: ---

If one subverts arm_acle.h and tries to create a minimal preprocessed test case
using ls64 builtins:

$ cat ls64.i
#pragma GCC aarch64 "arm_acle.h"
typedef __arm_data512_t data512_t;
void f(void *p, data512_t d) {
  __builtin_aarch64_st64b(p, d);
}
./xgcc -B . -c ls64.i -S -o /dev/null -march=armv8.7-a
ls64.i: In function ‘f’:
ls64.i:4:3: warning: implicit declaration of function ‘__builtin_aarch64_st64b’
[-Wimplicit-function-declaration]
    4 |   __builtin_aarch64_st64b(p, d);
      |   ^~~~~~~~~~~~~~~~~~~~~~~
ls64.i:4:3: warning: incompatible implicit declaration of built-in function
‘__builtin_aarch64_st64b’ [-Wbuiltin-declaration-mismatch]

we get these bogus warnings. The normal flow for e.g. Advanced SIMD builtins
is:
 - Backend calls add_builtin_function.
 - c_builtin_function adds the builtin to the list of visible_builtins.
 - push_file_scope gets called and builtins in visible_builtins are made
visible.

The problem here is that aarch64_init_ls64_builtins is called when processing
the pragma for arm_acle.h, which is *after* push_file_scope gets called, so the
builtins never get made visible by the C FE.

I think it might be easiest to make the ls64 handling more SVE-like and use the
simulate_builtin_function_decl langhook (which doesn't have this deferred
visibility situation), dropping the wrapper functions from arm_acle.h
altogether.

[Bug target/110132] aarch64: Bogus -Wbuiltin-declaration-mismatch with ls64 builtins

[acoplan at gcc dot gnu.org]

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

--- Comment #1 from Alex Coplan <acoplan at gcc dot gnu.org> ---
I should add, I believe (but haven't confirmed for sure) that the only reason
we don't see these warnings when including "arm_acle.h" is the warning
suppression that is done by the FE when parsing system headers.

[Bug target/110132] aarch64: Bogus -Wbuiltin-declaration-mismatch with ls64 builtins

[acoplan at gcc dot gnu.org]

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

Alex Coplan <acoplan at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |acoplan at gcc dot gnu.org
   Last reconfirmed|                            |2023-06-06
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1

--- Comment #2 from Alex Coplan <acoplan at gcc dot gnu.org> ---
Testing a patch.

[Bug target/110132] aarch64: Bogus -Wbuiltin-declaration-mismatch with ls64 builtins

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Alex Coplan <acoplan@gcc.gnu.org>:

https://gcc.gnu.org/g:9963029a24f2d2510b82e7106fae3f364da33c5d

commit r14-1617-g9963029a24f2d2510b82e7106fae3f364da33c5d
Author: Alex Coplan <alex.coplan@arm.com>
Date:   Tue Jun 6 15:19:03 2023 +0100

    aarch64: Allow compiler to define ls64 builtins [PR110132]

    This patch refactors the ls64 builtins to allow the compiler to define them
    directly instead of having wrapper functions in arm_acle.h. This should be
not
    only easier to maintain, but it makes two important correctness fixes:
     - It fixes PR110132, where the builtins ended up getting declared with
       invisible bindings in the C FE, so the FE ended up synthesizing
       incompatible implicit definitions for these builtins.
     - It allows the builtins to be used with LTO, which didn't work
previously.

    We also take the opportunity to add test coverage from C++ for these
    builtins.

    gcc/ChangeLog:

            PR target/110132
            * config/aarch64/aarch64-builtins.cc
(aarch64_general_simulate_builtin):
            New. Use it ...
            (aarch64_init_ls64_builtins): ... here. Switch to declaring public
ACLE
            names for builtins.
            (aarch64_general_init_builtins): Ensure we invoke the arm_acle.h
            setup if in_lto_p, just like we do for SVE.
            * config/aarch64/arm_acle.h: (__arm_ld64b): Delete.
            (__arm_st64b): Delete.
            (__arm_st64bv): Delete.
            (__arm_st64bv0): Delete.

    gcc/testsuite/ChangeLog:

            PR target/110132
            * lib/target-supports.exp
(check_effective_target_aarch64_asm_FUNC_ok):
            Extend to ls64.
            * g++.target/aarch64/acle/acle.exp: New.
            * g++.target/aarch64/acle/ls64.C: New test.
            * g++.target/aarch64/acle/ls64_lto.C: New test.
            * gcc.target/aarch64/acle/ls64_lto.c: New test.
            * gcc.target/aarch64/acle/pr110132.c: New test.

[Bug target/110132] aarch64: Bogus -Wbuiltin-declaration-mismatch with ls64 builtins

[acoplan at gcc dot gnu.org]

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

--- Comment #4 from Alex Coplan <acoplan at gcc dot gnu.org> ---
Fixed on trunk, keeping open for backports (I think we need this back to GCC
12).

[Bug target/110132] aarch64: Bogus -Wbuiltin-declaration-mismatch with ls64 builtins

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Alex Coplan
<acoplan@gcc.gnu.org>:

https://gcc.gnu.org/g:4eb01f987606e82ba4b7696f6cf79266d9e242ad

commit r13-7462-g4eb01f987606e82ba4b7696f6cf79266d9e242ad
Author: Alex Coplan <alex.coplan@arm.com>
Date:   Tue Jun 6 15:19:03 2023 +0100

    aarch64: Allow compiler to define ls64 builtins [PR110132]

    This patch refactors the ls64 builtins to allow the compiler to define them
    directly instead of having wrapper functions in arm_acle.h. This should be
not
    only easier to maintain, but it makes two important correctness fixes:
     - It fixes PR110132, where the builtins ended up getting declared with
       invisible bindings in the C FE, so the FE ended up synthesizing
       incompatible implicit definitions for these builtins.
     - It allows the builtins to be used with LTO, which didn't work
previously.

    We also take the opportunity to add test coverage from C++ for these
    builtins.

    gcc/ChangeLog:

            PR target/110132
            * config/aarch64/aarch64-builtins.cc
(aarch64_general_simulate_builtin):
            New. Use it ...
            (aarch64_init_ls64_builtins): ... here. Switch to declaring public
ACLE
            names for builtins.
            (aarch64_general_init_builtins): Ensure we invoke the arm_acle.h
            setup if in_lto_p, just like we do for SVE.
            * config/aarch64/arm_acle.h: (__arm_ld64b): Delete.
            (__arm_st64b): Delete.
            (__arm_st64bv): Delete.
            (__arm_st64bv0): Delete.

    gcc/testsuite/ChangeLog:

            PR target/110132
            * lib/target-supports.exp
(check_effective_target_aarch64_asm_FUNC_ok):
            Extend to ls64.
            * g++.target/aarch64/acle/acle.exp: New.
            * g++.target/aarch64/acle/ls64.C: New test.
            * g++.target/aarch64/acle/ls64_lto.C: New test.
            * gcc.target/aarch64/acle/ls64_lto.c: New test.
            * gcc.target/aarch64/acle/pr110132.c: New test.

    (cherry picked from commit 9963029a24f2d2510b82e7106fae3f364da33c5d)

[Bug target/110132] aarch64: Bogus -Wbuiltin-declaration-mismatch with ls64 builtins

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Alex Coplan
<acoplan@gcc.gnu.org>:

https://gcc.gnu.org/g:4481d70c9edcd89a8d9f6c0d705b05230aa080e3

commit r12-9720-g4481d70c9edcd89a8d9f6c0d705b05230aa080e3
Author: Alex Coplan <alex.coplan@arm.com>
Date:   Tue Jun 6 15:19:03 2023 +0100

    aarch64: Allow compiler to define ls64 builtins [PR110132]

    This patch refactors the ls64 builtins to allow the compiler to define them
    directly instead of having wrapper functions in arm_acle.h. This should be
not
    only easier to maintain, but it makes two important correctness fixes:
     - It fixes PR110132, where the builtins ended up getting declared with
       invisible bindings in the C FE, so the FE ended up synthesizing
       incompatible implicit definitions for these builtins.
     - It allows the builtins to be used with LTO, which didn't work
previously.

    We also take the opportunity to add test coverage from C++ for these
    builtins.

    gcc/ChangeLog:

            PR target/110132
            * config/aarch64/aarch64-builtins.cc
(aarch64_general_simulate_builtin):
            New. Use it ...
            (aarch64_init_ls64_builtins): ... here. Switch to declaring public
ACLE
            names for builtins.
            (aarch64_general_init_builtins): Ensure we invoke the arm_acle.h
            setup if in_lto_p, just like we do for SVE.
            * config/aarch64/arm_acle.h: (__arm_ld64b): Delete.
            (__arm_st64b): Delete.
            (__arm_st64bv): Delete.
            (__arm_st64bv0): Delete.

    gcc/testsuite/ChangeLog:

            PR target/110132
            * lib/target-supports.exp
(check_effective_target_aarch64_asm_FUNC_ok):
            Extend to ls64.
            * g++.target/aarch64/acle/acle.exp: New.
            * g++.target/aarch64/acle/ls64.C: New test.
            * g++.target/aarch64/acle/ls64_lto.C: New test.
            * gcc.target/aarch64/acle/ls64_lto.c: New test.
            * gcc.target/aarch64/acle/pr110132.c: New test.

    (cherry picked from commit 9963029a24f2d2510b82e7106fae3f364da33c5d)

[Bug target/110132] aarch64: Bogus -Wbuiltin-declaration-mismatch with ls64 builtins

[acoplan at gcc dot gnu.org]

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

Alex Coplan <acoplan at gcc dot gnu.org> changed:

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

--- Comment #7 from Alex Coplan <acoplan at gcc dot gnu.org> ---
Fixed on all affected branches.