On Mon, Mar 8, 2021 at 12:44 PM Alex Coplan via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > Hi all, > > As discussed in the PR, we currently have two different numbering > schemes for SVE builtins: one for C, and one for C++. This is > problematic for LTO, where we end up getting confused about which > intrinsic we're talking about. This patch inserts placeholders into the > registered_functions vector to ensure that there is a consistent > numbering scheme for both C and C++. > > Initially I tried using a null decl for these placeholders, but this > trips up some validation when the builtin trees are streamed into lto1,
Care to elaborate? Note we stream builtins by their function code and thus expect to be able to materialize them later - materializing to NULL obviously fails but then the issue is the proper decl isn't there in that case. Materializing a "dummy" decl just will delay the inevitable breakage. > so I ended up building dummy FUNCTION_DECLs as placeholders. > > To get better test coverage here, I'm wondering if we could make some of > the existing ACLE tests also run with LTO? Perhaps as a follow-on patch? > For now I've just added the specific testcase from the PR. > > Bootstrapped and regtested on aarch64-linux-gnu, OK for trunk? > > Thanks, > Alex > > --- > > gcc/ChangeLog: > > PR target/99216 > * config/aarch64/aarch64-sve-builtins.cc > (function_builder::add_function): Add placeholder_p argument, build > placeholder decls if this is set. > (function_builder::add_unique_function): Instead of conditionally > adding > direct overloads, unconditionally add either a direct overload or a > placeholder. > (function_builder::add_overloaded_function): Set placeholder_p if > we're > using C++ overloads. > (function_builder::add_overloaded_functions): Don't return early for > m_direct_overloads: we need to add placeholders. > * config/aarch64/aarch64-sve-builtins.h > (function_builder::add_function): Add placeholder_p argument. > > gcc/testsuite/ChangeLog: > > PR target/99216 > * g++.target/aarch64/sve/pr99216.C: New test.
