Skip to content

[Clang] Add __has_target_builtin macro #126324

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

[Clang] Add __has_target_builtin macro #126324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
46cce74
[Clang] Add __has_target_builtin macro
sarnex Feb 7, 2025
2a2a18b
add release note, only define for offloading targets, update test
sarnex Feb 10, 2025
644ea85
doc feedback
sarnex Feb 10, 2025
887de75
improve comparison doc
sarnex Feb 10, 2025
7781516
space
sarnex Feb 10, 2025
5d84bc5
spacing again
sarnex Feb 10, 2025
7e2dc73
missed endif
sarnex Feb 10, 2025
2668404
make macro defined on all targets, address aarons feedback
sarnex Feb 11, 2025
7143389
improve example
sarnex Feb 12, 2025
6682ae2
add comment
sarnex Feb 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions clang/docs/LanguageExtensions.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ It can be used like this:
``__has_builtin`` should not be used to detect support for a builtin macro;
use ``#ifdef`` instead.

When using device offloading, a builtin is considered available if it is
available on either the host or the device targets.
Use ``__has_target_builtin`` to consider only the current target.

``__has_constexpr_builtin``
---------------------------

Expand Down Expand Up @@ -96,6 +100,55 @@ the ``<cmath>`` header file to conditionally make a function constexpr whenever
the constant evaluation of the corresponding builtin (for example,
``std::fmax`` calls ``__builtin_fmax``) is supported in Clang.

``__has_target_builtin``
------------------------

This function-like macro takes a single identifier argument that is the name of
a builtin function, a builtin pseudo-function (taking one or more type
arguments), or a builtin template.
It evaluates to 1 if the builtin is supported on the current target or 0 if not.

``__has_builtin`` and ``__has_target_builtin`` behave identically for normal C++ compilations.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this means users should use __has_target_builtin in place of __has_builtin so that their code will correctly check for the builtin on host or device?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably better to be portable with GCC for average users, but it's the recommended solution for code that's intended to be run on the GPU I'd say.

Copy link
Member Author

@sarnex sarnex Feb 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the user's goal is to check that the builtin can be codegen'd on the current target being compiled, then yes they should use __has_target_builtin. if they want to confirm it can be parsed but not necessarily codegen'd, then they should use __has_builtin. if that's unclear let me know and i can try to improve the doc

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the user's goal is to check that the builtin can be codegen'd on the current target being compiled, then yes they should use __has_target_builtin. if they want to confirm it can be parsed but not necessarily codegen'd, then they should use __has_builtin. if that's unclear let me know and i can try to improve the doc

Users don't typically think in terms of "parsed" and "codegenned", but more "works" and "doesn't work", which I think means "codegenned" in general. The line we're drawing here is pretty subtle, so perhaps more real world examples would help; it's hard to understand why a builtin can be parsed but cannot be used.

Copy link
Member Author

@sarnex sarnex Feb 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah that's a fair point, so it sounds like you'd prefer an example of when codegen doesn't work even though the user is checking with has_builtin, and for that i could show the motiving example for this change which is something like

void foo() {
#if __has_builtin(__builtin_ia32_pause)
__builtin_ia32_pause();
#else
abort();
#endif
}

and if the current target is an offloading target (amdgpu/nvptx/spirv) and the aux target is x86, we will get a error saying it can't be codegen'd

would that kind of example address your concern? thx

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think it would, along with the prose explaining why that's not a bug but is actually by design for __has_builtin. I think users would reasonably look at that and say it should not be an error, so it'd be nice to make sure they understand the intent.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, sounds good.

@yxsamliu @Artem-B would implementing Aaron's suggestion address your concerns as well?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to including a concrete example with an explanation. The fact that a builtin may be both visible but not usable will be a surprise for a C++ user.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, ill add this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added in the latest commit, please take a look and let me know if it's clear


For heterogeneous compilations that see source code intended for more than one target:

``__has_builtin`` returns true if the builtin is known to the compiler
(i.e. it's available via one of the targets), but makes no promises whether it's available on the current target.
The compiler can parse it, but not necessarily generate code for it.

``__has_target_builtin`` returns true if the builtin can actually be generated for the current target.

As a motivating example, let's try to guard a builtin call using ``__has_builtin`` in a heterogeneous compilation,
such as OpenMP Offloading, with the host being x86-64 and the offloading device being AMDGPU.

.. code-block:: c++

#if __has_builtin(__builtin_ia32_pause)
__builtin_ia32_pause();
#else
abort();
#endif

Compilation of this code results in a compiler error because ``__builtin_ia32_pause`` is known to the compiler because
it is a builtin supported by the host x86-64 compilation so ``__has_builtin`` returns true. However, code cannot
be generated for ``__builtin_ia32_pause`` during the offload AMDGPU compilation as it is not supported on that target.
Comment on lines +132 to +134
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, this is helping somewhat. But I think a user is still going to ask themselves "why does __has_builtin return true in this case?". The host and the offload are separate compilations, so logically, it stands to reason that __has_builtin would return true for the host and false for the offload. And because of:

__has_builtin and __has_target_builtin behave identically for normal C++ compilations.

They're going to wonder why they wouldn't just replace all uses of __has_builtin with __has_target_builtin, which begs the question of why __has_builtin isn't just being "fixed".

(I'll be honest, I still don't fully understand the answer to that myself.)


To guard uses of builtins in heterogeneous compilations such as this,
``__has_target_builtin`` can be used as per the below example to verify code can be generated
for the referenced builtin on the current target being compiled.

.. code-block:: c++

#if __has_target_builtin(__builtin_ia32_pause)
__builtin_ia32_pause();
#else
abort();
#endif

.. note::
``__has_target_builtin`` should not be used to detect support for a builtin macro;
use ``#ifdef`` instead.

.. _langext-__has_feature-__has_extension:

``__has_feature`` and ``__has_extension``
Expand Down
6 changes: 6 additions & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,12 @@ Non-comprehensive list of changes in this release
New Compiler Flags
------------------

New Compiler Builtins
---------------------

- The new ``__has_target_builtin`` macro can be used to check if a builtin is available
on the current target.

Deprecated Compiler Flags
-------------------------

Expand Down
1 change: 1 addition & 0 deletions clang/include/clang/Lex/Preprocessor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ class Preprocessor {
IdentifierInfo *Ident__has_extension; // __has_extension
IdentifierInfo *Ident__has_builtin; // __has_builtin
IdentifierInfo *Ident__has_constexpr_builtin; // __has_constexpr_builtin
IdentifierInfo *Ident__has_target_builtin; // __has_target_builtin
IdentifierInfo *Ident__has_attribute; // __has_attribute
IdentifierInfo *Ident__has_embed; // __has_embed
IdentifierInfo *Ident__has_include; // __has_include
Expand Down
19 changes: 14 additions & 5 deletions clang/lib/Lex/PPMacroExpansion.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,7 @@ void Preprocessor::RegisterBuiltinMacros() {
Ident__has_builtin = RegisterBuiltinMacro("__has_builtin");
Ident__has_constexpr_builtin =
RegisterBuiltinMacro("__has_constexpr_builtin");
Ident__has_target_builtin = RegisterBuiltinMacro("__has_target_builtin");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we may want to define this macro for offloading languages only. The reason is that non-offloading languages do not need this macro but if they start to use this macro then it will break again in offloading languages like __has_builtin did.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the usage would commonly be

#if defined(__has_target_builtin) && __has_target_builtin(foo)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My fear is that some C++ library headers start to use this macro __has_target_builtin in place of __has_builtin, and we cannot modify such headers.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, will do this. I can't find a good way to detect offloading languages in general here, so I'm just going to check for CUDA/HIP/SYCLDevice/OpenMPDevice, let me know if there's some common logic I can rely on that I missed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in latest commit

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My fear is that some C++ library headers start to use this macro __has_target_builtin in place of __has_builtin, and we cannot modify such headers.

IMO, now that we do document semantics of __has_target_builtin(), its misuse on the library side will be their problem to fix. The problem with __has_builtin() was that it was never intended to handle heterogeneous compilation, and that's what created the issue when CUDA/HIP made builtins from both host and device visible to the compiler, but not all of them codegen-able. __has_target_builtin() clearly states what to expect. Sure, it's possible to misuse it, but having it available unconditionally will make it much less cumbersome to use in the headers shared between CUDA and C++, and that's a fairly common use case.

I'd prefer to have __has_target_builtin() generally available.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, __has_target_builtin() is probably identical to __has_builtin on non-offloading related things. It's up to them if they keep it portable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

made it unconditional again in the latest commit

i kept the code example in langref because probably we still want to recommend only using this for offloading targets, even though it will work on non-offloading targets. let met know if you disagree.

Ident__has_attribute = RegisterBuiltinMacro("__has_attribute");
if (!getLangOpts().CPlusPlus)
Ident__has_c_attribute = RegisterBuiltinMacro("__has_c_attribute");
Expand Down Expand Up @@ -1797,16 +1798,18 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {
diag::err_feature_check_malformed);
return II && HasExtension(*this, II->getName());
});
} else if (II == Ident__has_builtin) {
} else if (II == Ident__has_builtin || II == Ident__has_target_builtin) {
bool IsHasTargetBuiltin = II == Ident__has_target_builtin;
EvaluateFeatureLikeBuiltinMacro(
OS, Tok, II, *this, false,
[this](Token &Tok, bool &HasLexedNextToken) -> int {
[this, IsHasTargetBuiltin](Token &Tok, bool &HasLexedNextToken) -> int {
IdentifierInfo *II = ExpectFeatureIdentifierInfo(
Tok, *this, diag::err_feature_check_malformed);
if (!II)
return false;
else if (II->getBuiltinID() != 0) {
switch (II->getBuiltinID()) {
unsigned BuiltinID = II->getBuiltinID();
if (BuiltinID != 0) {
switch (BuiltinID) {
case Builtin::BI__builtin_cpu_is:
return getTargetInfo().supportsCpuIs();
case Builtin::BI__builtin_cpu_init:
Expand All @@ -1819,8 +1822,14 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {
// usual allocation and deallocation functions. Required by libc++
return 201802;
default:
// __has_target_builtin should return false for aux builtins.
// isAuxBuiltinID only returns true if the builtin is
// supported by the aux target alone.
if (IsHasTargetBuiltin &&
getBuiltinInfo().isAuxBuiltinID(BuiltinID))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic doesn't seem right to me. What happens if this builtin is supported on BOTH the host and device here? Shouldn't we still return 'true'?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. The function name is isAuxBuiltinID but the implementation of it makes it so that the behavior is like isOnlyAuxBuiltinID. The implementation of the function is:

 /// Return true if builtin ID belongs to AuxTarget.
 bool isAuxBuiltinID(unsigned ID) const {  
   return ID >= (Builtin::FirstTSBuiltin + NumTargetBuiltins); 
  }

When we register builtins, we do it like this:

TargetShards = Target.getTargetBuiltins();
  for (const auto &Shard : TargetShards)
    NumTargetBuiltins += Shard.Infos.size();
  if (AuxTarget) {
    AuxTargetShards = AuxTarget->getTargetBuiltins();
    for (const auto &Shard : AuxTargetShards)
      NumAuxTargetBuiltins += Shard.Infos.size();
  }
}

So we register all the target builtins before the aux target builtins, and we only consider something an aux builtin if it was registered specifically as an aux builtin, so I think the logic does what we want.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! Neat! Can you document that somewhere on this? Just extending the comment here will be really helpful to the next person along.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, opened PR here and added you as a reviewer.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! Thanks! I meant here as well though, it was REALLY jarring to me and I suspect next one here will ahve the same problem.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah sure, will update this pr as well

return false;
return Builtin::evaluateRequiredTargetFeatures(
getBuiltinInfo().getRequiredFeatures(II->getBuiltinID()),
getBuiltinInfo().getRequiredFeatures(BuiltinID),
getTargetInfo().getTargetOpts().FeatureMap);
}
return true;
Expand Down
24 changes: 24 additions & 0 deletions clang/test/Preprocessor/has_target_builtin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// RUN: %clang_cc1 -fopenmp -triple=spirv64 -fopenmp-is-target-device \
// RUN: -aux-triple x86_64-linux-unknown -E %s | FileCheck -implicit-check-not=HAS %s

// RUN: %clang_cc1 -fopenmp -triple=nvptx64 -fopenmp-is-target-device \
// RUN: -aux-triple x86_64-linux-unknown -E %s | FileCheck -implicit-check-not=HAS %s

// RUN: %clang_cc1 -fopenmp -triple=amdgcn-amd-amdhsa -fopenmp-is-target-device \
// RUN: -aux-triple x86_64-linux-unknown -E %s | FileCheck -implicit-check-not=HAS %s

// RUN: %clang_cc1 -fopenmp -triple=aarch64 -fopenmp-is-target-device \
// RUN: -aux-triple x86_64-linux-unknown -E %s | FileCheck -implicit-check-not=HAS %s

// RUN: %clang_cc1 -triple=aarch64 -E %s | FileCheck -implicit-check-not=HAS %s

// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -E %s | \
// RUN: FileCheck -check-prefix=CHECK-NO-OFFLOAD-HAS-BUILTIN -implicit-check-not=DOESNT %s

// CHECK: DOESNT
// CHECK-NO-OFFLOAD-HAS-BUILTIN: HAS
#if __has_target_builtin(__builtin_ia32_pause)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also add test coverage for when the target does have the builtin?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in latest commit, thx

HAS
#else
DOESNT
#endif