]> git.ipfire.org Git - thirdparty/gcc.git/commit
c++: Allow overloaded builtins to be used in SFINAE context
authorMatthew Malcomson <mmalcomson@nvidia.com>
Mon, 7 Oct 2024 15:42:41 +0000 (16:42 +0100)
committerMatthew Malcomson <mmalcomson@nvidia.com>
Mon, 9 Dec 2024 14:05:50 +0000 (14:05 +0000)
commit9ed094a817ecaf43c79505286759b88eb0555be2
tree3dd49e1f4edef2fb100ceafd64afd729918702c7
parent548afd73cdbf310403a1e3f34226372c16c29706
c++: Allow overloaded builtins to be used in SFINAE context

This commit newly introduces the ability to use overloaded builtins in
C++ SFINAE context.

The goal behind this is in order to ensure there is a single mechanism
that libstdc++ can use to determine whether a given type can be used in
the atomic fetch_add (and similar) builtins.  I am working on another
patch that hopes to use this mechanism to identify whether fetch_add
(and similar) work on floating point types.

Current state of the world:

    GCC currently exposes resolved versions of these builtins to the
    user, so for GCC it's currently possible to use tests similar to the
    below to check for atomic loads on a 2 byte sized object.
      #if __has_builtin(__atomic_load_2)
    Clang does not expose resolved versions of the atomic builtins.

    clang currently allows SFINAE on builtins, so that C++ code can
    check whether a builtin is available on a given type.
    GCC does not (and that is what this patch aims to change).

    C libraries like libatomic can check whether a given atomic builtin
    can work on a given type by using autoconf to check for a
    miscompilation when attempting such a use.

My goal:
    I would like to enable floating point fetch_add (and similar) in
    GCC, in order to use those overloads in libstdc++ implementation of
    atomic<float>::fetch_add.
    This should allow compilers targeting GPU's which have floating
    point fetch_add instructions to emit optimal code.

    In order to do that I need some consistent mechanism that libstdc++
    can use to identify whether the fetch_add builtins have floating
    point overloads (and for which types these exist).

    I would hence like to enable SFINAE on builtins, so that libstdc++
    can use that mechanism for the floating point fetch_add builtins.

Implementation follows the existing mechanism for handling SFINAE
contexts in c-common.cc.  A boolean is passed into the c-common.cc
function indicating whether these functions should emit errors or not.
This boolean comes from `complain & tf_error` in the C++ frontend.
(Similar to other functions like valid_array_size_p and
c_build_vec_perm_expr).

This is done both for resolve_overloaded_builtin and
check_builtin_function_arguments, both of which can be used in SFINAE
contexts.
    I attempted to trigger something using the `reject_gcc_builtin`
    function in an SFINAE context.  Given the context where this
    function is called from the C++ frontend it looks like it may be
    possible, but I did not manage to trigger this in template context
    by attempting to do something similar to the testcases added around
    those calls.
    - I would appreciate any feedback on whether this is something that
      can happen in a template context, and if so some help writing a
      relevant testcase for it.

Both of these functions have target hooks for target specific builtins
that I have updated to take the extra boolean flag.  I have not adjusted
the functions implementing those target hooks (except to update the
declarations) so target specific builtins will still error in SFINAE
contexts.
- I could imagine not updating the target hook definition since nothing
  would use that change.  However I figure that allowing targets to
  decide this behaviour would be the right thing to do eventually, and
  since this is the target-independent part of the change to do that
  this patch should make that change.
  Could adjust if others disagree.

Other relevant points that I'd appreciate reviewers check:
- I did not pass this new flag through
  atomic_bitint_fetch_using_cas_loop since the _BitInt type is not
  available in the C++ frontend and I didn't want if conditions that can
  not be executed in the source.
- I only test non-compile-time-constant types with SVE types, since I do
  not know of a way to get a VLA into a SFINAE context.
- While writing tests I noticed a few differences with clang in this
  area.  I don't think they are problematic but am mentioning them for
  completeness and to allow others to judge if these are a problem).
  - atomic_fetch_add on a boolean is allowed by clang.
  - When __atomic_load is passed an invalid memory model (i.e. too
    large), we give an SFINAE failure while clang does not.

Bootstrap and regression tested on AArch64 and x86_64.
Built first stage on targets whose target hook declaration needed
updated (though did not regtest etc).  Targets triplets I built in order
to check the backend specific changes I made:
   - arm-none-linux-gnueabihf
   - avr-linux-gnu
   - riscv-linux-gnu
   - powerpc-linux-gnu
   - s390x-linux-gnu

Ok for commit to trunk?

gcc/c-family/ChangeLog:

* c-common.cc (builtin_function_validate_nargs,
check_builtin_function_arguments,
speculation_safe_value_resolve_call,
speculation_safe_value_resolve_params, sync_resolve_size,
sync_resolve_params, get_atomic_generic_size,
resolve_overloaded_atomic_exchange,
resolve_overloaded_atomic_compare_exchange,
resolve_overloaded_atomic_load, resolve_overloaded_atomic_store,
resolve_overloaded_builtin):  Add `complain` boolean parameter
and determine whether to emit errors based on its value.
* c-common.h (check_builtin_function_arguments,
resolve_overloaded_builtin):  Mention `complain` boolean
parameter in declarations.  Give it a default of `true`.

gcc/ChangeLog:

* config/aarch64/aarch64-c.cc
(aarch64_resolve_overloaded_builtin,aarch64_check_builtin_call):
Add new unused boolean parameter to match target hook
definition.
* config/arm/arm-builtins.cc (arm_check_builtin_call): Likewise.
* config/arm/arm-c.cc (arm_resolve_overloaded_builtin):
Likewise.
* config/arm/arm-protos.h (arm_check_builtin_call): Likewise.
* config/avr/avr-c.cc (avr_resolve_overloaded_builtin):
Likewise.
* config/riscv/riscv-c.cc (riscv_check_builtin_call,
riscv_resolve_overloaded_builtin): Likewise.
* config/rs6000/rs6000-c.cc (altivec_resolve_overloaded_builtin):
Likewise.
* config/rs6000/rs6000-protos.h (altivec_resolve_overloaded_builtin):
Likewise.
* config/s390/s390-c.cc (s390_resolve_overloaded_builtin):
Likewise.
* doc/tm.texi: Regenerate.
* target.def (TARGET_RESOLVE_OVERLOADED_BUILTIN,
TARGET_CHECK_BUILTIN_CALL): Update prototype to include a
boolean parameter that indicates whether errors should be
emitted.  Update documentation to mention this fact.

gcc/cp/ChangeLog:

* call.cc (build_cxx_call):  Pass `complain` parameter to
check_builtin_function_arguments.  Take its value from the
`tsubst_flags_t` type `complain & tf_error`.
* semantics.cc (finish_call_expr):  Pass `complain` parameter to
resolve_overloaded_builtin.  Take its value from the
`tsubst_flags_t` type `complain & tf_error`.

gcc/testsuite/ChangeLog:

* g++.dg/template/builtin-atomic-overloads.def: New test.
* g++.dg/template/builtin-atomic-overloads1.C: New test.
* g++.dg/template/builtin-atomic-overloads2.C: New test.
* g++.dg/template/builtin-atomic-overloads3.C: New test.
* g++.dg/template/builtin-atomic-overloads4.C: New test.
* g++.dg/template/builtin-atomic-overloads5.C: New test.
* g++.dg/template/builtin-atomic-overloads6.C: New test.
* g++.dg/template/builtin-atomic-overloads7.C: New test.
* g++.dg/template/builtin-atomic-overloads8.C: New test.
* g++.dg/template/builtin-sfinae-check-function-arguments.C: New test.
* g++.dg/template/builtin-speculation-overloads.def: New test.
* g++.dg/template/builtin-speculation-overloads1.C: New test.
* g++.dg/template/builtin-speculation-overloads2.C: New test.
* g++.dg/template/builtin-speculation-overloads3.C: New test.
* g++.dg/template/builtin-speculation-overloads4.C: New test.
* g++.dg/template/builtin-speculation-overloads5.C: New test.
* g++.dg/template/builtin-validate-nargs.C: New test.

Signed-off-by: Matthew Malcomson <mmalcomson@nvidia.com>
32 files changed:
gcc/c-family/c-common.cc
gcc/c-family/c-common.h
gcc/config/aarch64/aarch64-c.cc
gcc/config/arm/arm-builtins.cc
gcc/config/arm/arm-c.cc
gcc/config/arm/arm-protos.h
gcc/config/avr/avr-c.cc
gcc/config/riscv/riscv-c.cc
gcc/config/rs6000/rs6000-c.cc
gcc/config/rs6000/rs6000-protos.h
gcc/config/s390/s390-c.cc
gcc/cp/call.cc
gcc/cp/semantics.cc
gcc/doc/tm.texi
gcc/target.def
gcc/testsuite/g++.dg/template/builtin-atomic-overloads.def [new file with mode: 0644]
gcc/testsuite/g++.dg/template/builtin-atomic-overloads1.C [new file with mode: 0644]
gcc/testsuite/g++.dg/template/builtin-atomic-overloads2.C [new file with mode: 0644]
gcc/testsuite/g++.dg/template/builtin-atomic-overloads3.C [new file with mode: 0644]
gcc/testsuite/g++.dg/template/builtin-atomic-overloads4.C [new file with mode: 0644]
gcc/testsuite/g++.dg/template/builtin-atomic-overloads5.C [new file with mode: 0644]
gcc/testsuite/g++.dg/template/builtin-atomic-overloads6.C [new file with mode: 0644]
gcc/testsuite/g++.dg/template/builtin-atomic-overloads7.C [new file with mode: 0644]
gcc/testsuite/g++.dg/template/builtin-atomic-overloads8.C [new file with mode: 0644]
gcc/testsuite/g++.dg/template/builtin-sfinae-check-function-arguments.C [new file with mode: 0644]
gcc/testsuite/g++.dg/template/builtin-speculation-overloads.def [new file with mode: 0644]
gcc/testsuite/g++.dg/template/builtin-speculation-overloads1.C [new file with mode: 0644]
gcc/testsuite/g++.dg/template/builtin-speculation-overloads2.C [new file with mode: 0644]
gcc/testsuite/g++.dg/template/builtin-speculation-overloads3.C [new file with mode: 0644]
gcc/testsuite/g++.dg/template/builtin-speculation-overloads4.C [new file with mode: 0644]
gcc/testsuite/g++.dg/template/builtin-speculation-overloads5.C [new file with mode: 0644]
gcc/testsuite/g++.dg/template/builtin-validate-nargs.C [new file with mode: 0644]