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.

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
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
36 changes: 36 additions & 0 deletions clang/docs/LanguageExtensions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ 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 for an
offloading target.

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

Expand Down Expand Up @@ -96,6 +101,37 @@ 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.
The behavior is different than ``__has_builtin`` when there is an auxiliary target,
such when offloading to a target device.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
such when offloading to a target device.
such as when offloading to a target device.

Copy link
Member Author

Choose a reason for hiding this comment

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

cant english today, thanks

Copy link
Member

Choose a reason for hiding this comment

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

I'd rephrase it to be more specific in terms of what the difference is rather than when it occurs.

__has_builtin() and __has_target_builtin() behave identically for normal C++ compilations.
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. We can parse it, but not necessarily codegen it.
  • __has_target_builtin() returns true if the builtin can actually be codegen'ed for the current target.

__has_target_builtin() is, effectively, functional superset of CUDA's __CUDA_ARCH__ -- it allows distinguishing both host and target architectures. It has to be treated with similar caution so it does not break consistency of the TU source code seen by the compiler across sub-compilations.

Copy link
Member Author

@sarnex sarnex Feb 10, 2025

Choose a reason for hiding this comment

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

thanks, i like the way you worded it so i'll use most of this verbatim

It can be used like this:

.. code-block:: c++

#ifndef __has_target_builtin // Optional of course.
#define __has_target_builtin(x) 0 // Compatibility with non-clang compilers.
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be more helpful to do something like ifdef CUDA ... else __has_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.

hopefully the latest commit has the use case youre looking for

Copy link
Collaborator

Choose a reason for hiding this comment

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

if we make it available to C++, we'd better document the following invalid usage which originally leads to this extension:

 #if !__has_target_builtin(__wfi) 
 static __inline__ void __attribute__((__always_inline__, __nodebug__)) __wfi(void) { 
   __builtin_arm_wfi(); 
 } 
 #endif 

we should emphasize that a C++ header may be used by offloading languages, and in offloading language, the same source is compiled for host and device target separately. A builtin not available for the current target does not justify defining the builtin for both host and device targets. In this case, better to use __has_builtin(__wfi) since it makes sure the condition is true for both hosts and device targets so that the code won't break when used in offloading languages.

Copy link
Member

Choose a reason for hiding this comment

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

We have somewhat conflicting requirements:

  • On C++ side, writers do not care about offloading (and we can't force them to). They only have __has_builtin() and it does what they need -- if the given builtin exists it will be compileable.
  • On offloading side, we want C++ headers to work out of the box for the host side. Ideally with the host and device compilations seeing the same code after preprocessing, and that's where we get into this problem. We can't tell whether the original C++ code needs __has_builtin() (works well enough for most uses inside of host function bodies) or if it needs __has_target_builtin() (e.g. when it's used inside a lambda or constexpr function which is implicitly HD, and we do need to generate code for it).

I'm not sure we can find a universal solution. That said, __has_target_builtin() gives us some flexibility on the offloading side. C++ side should stick with __has_builtin(). __has_target_builtin() should only be used when offloading comes into the picture, but it includes the possibility that it will be used in the headers shared with C++ and therefore the builtin itself should be available there.


...
#if __has_target_builtin(__builtin_trap)
__builtin_trap();
#else
abort();
#endif
...

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

``__has_target_built`` is only defined for offloading targets.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
``__has_target_built`` is only defined for offloading targets.
``__has_target_builtin`` is only defined for offloading targets.

Copy link
Member Author

Choose a reason for hiding this comment

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

wow thats embarrassing, thanks


.. _langext-__has_feature-__has_extension:

``__has_feature`` and ``__has_extension``
Expand Down
6 changes: 6 additions & 0 deletions clang/docs/ReleaseNotes.rst
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 offloading target.

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

Expand Down
1 change: 1 addition & 0 deletions clang/include/clang/Lex/Preprocessor.h
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
22 changes: 17 additions & 5 deletions clang/lib/Lex/PPMacroExpansion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,12 @@ void Preprocessor::RegisterBuiltinMacros() {
Ident__has_builtin = RegisterBuiltinMacro("__has_builtin");
Ident__has_constexpr_builtin =
RegisterBuiltinMacro("__has_constexpr_builtin");
if (getLangOpts().OpenMPIsTargetDevice || getLangOpts().CUDAIsDevice ||
getLangOpts().SYCLIsDevice)
Ident__has_target_builtin = RegisterBuiltinMacro("__has_target_builtin");
else
Ident__has_target_builtin = nullptr;

Ident__has_attribute = RegisterBuiltinMacro("__has_attribute");
if (!getLangOpts().CPlusPlus)
Ident__has_c_attribute = RegisterBuiltinMacro("__has_c_attribute");
Expand Down Expand Up @@ -1797,16 +1803,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()) {
auto BuiltinID = II->getBuiltinID();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please spell out the type explicitly.

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

if (BuiltinID != 0) {
switch (BuiltinID) {
case Builtin::BI__builtin_cpu_is:
return getTargetInfo().supportsCpuIs();
case Builtin::BI__builtin_cpu_init:
Expand All @@ -1819,8 +1827,12 @@ 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.
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
27 changes: 27 additions & 0 deletions clang/test/Preprocessor/has_target_builtin.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// RUN: %clang_cc1 -fopenmp -triple=spirv64 -fopenmp-is-target-device \
// RUN: -aux-triple x86_64-linux-unknown -E %s | FileCheck -implicit-check-not="{{BAD|DOESNT}}" %s

// RUN: %clang_cc1 -fopenmp -triple=nvptx64 -fopenmp-is-target-device \
// RUN: -aux-triple x86_64-linux-unknown -E %s | FileCheck -implicit-check-not="{{BAD|DOESNT}}" %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="{{BAD|DOESNT}}" %s

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

// RUN: %clang_cc1 -triple=aarch64 -E %s | FileCheck -check-prefix=CHECK-NOTOFFLOAD -implicit-check-not="{{GOOD|HAS|BAD}}" %s

// CHECK: GOOD

// CHECK-NOTOFFLOAD: DOESNT
#ifdef __has_target_builtin
HAS
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should probably check for HAS explicitly.

Copy link
Member Author

@sarnex sarnex Feb 11, 2025

Choose a reason for hiding this comment

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

the macro is unconditionally available in the latest commit so i removed the checking for the macro being defined

#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

BAD
#else
GOOD
#endif
#else
DOESNT
#endif
Loading