-
Notifications
You must be signed in to change notification settings - Fork 54
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
Remove request for preprocessor guards from header files. #321
Changes from 10 commits
fc50eb1
994aec6
1270932
6e3991a
9079875
ae42458
92e5b8c
fb4b4f8
15bd063
4fafea9
50f5670
9d357be
cd750bf
7b3ee34
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -925,8 +925,9 @@ and: | |
to the more specific header files below. These intrinsics are in the | ||
C implementation namespace and begin with double underscores. It is | ||
unspecified whether they are available without the header being | ||
included. The `__ARM_ACLE` macro should be tested before including the | ||
header: | ||
included. The `__ARM_ACLE` macro can be tested before including the header. | ||
However, this is not recommended for cases where target feature selection is | ||
overridden within code, for example, when using #function-multi-versioning. | ||
|
||
``` c | ||
#ifdef __ARM_ACLE | ||
|
@@ -939,8 +940,10 @@ header: | |
`<arm_fp16.h>` is provided to define the scalar 16-bit floating point | ||
arithmetic intrinsics. As these intrinsics are in the user namespace, | ||
an implementation would not normally define them until the header is | ||
included. The `__ARM_FEATURE_FP16_SCALAR_ARITHMETIC` feature macro | ||
should be tested before including the header: | ||
included. The `__ARM_FEATURE_FP16_SCALAR_ARITHMETIC` macro can be tested before | ||
including the header. However, this is not recommended for cases where target | ||
feature selection is overridden within code, for example, when using | ||
#function-multi-versioning. | ||
|
||
``` c | ||
#ifdef __ARM_FEATURE_FP16_SCALAR_ARITHMETIC | ||
|
@@ -953,8 +956,10 @@ should be tested before including the header: | |
`<arm_bf16.h>` is provided to define the 16-bit brain floating point | ||
arithmetic intrinsics. As these intrinsics are in the user namespace, | ||
an implementation would not normally define them until the header is | ||
included. The `__ARM_FEATURE_BF16` feature macro | ||
should be tested before including the header: | ||
included. The `__ARM_FEATURE_BF16` macro can be tested before including the | ||
header. However, this is not recommended for cases where target feature | ||
selection is overridden within code, for example, when using | ||
function-multi-versioning. | ||
|
||
``` c | ||
#ifdef __ARM_FEATURE_BF16 | ||
|
@@ -975,8 +980,11 @@ instructions available are conversion intrinsics between `bfloat16_t` and | |
intrinsics](#advanced-simd-neon-intrinsics) and associated | ||
[data types](#vector-data-types). As these intrinsics and data types are | ||
in the user namespace, an implementation would not normally define them | ||
until the header is included. The `__ARM_NEON` macro should be tested | ||
before including the header: | ||
until the header is included. The `__ARM_NEON` macro can be tested before | ||
including the header. However, this is not recommended for cases where target | ||
feature selection is overridden within code, for example, when using | ||
#function-multi-versioning. | ||
|
||
|
||
``` c | ||
#ifdef __ARM_NEON | ||
|
@@ -999,8 +1007,9 @@ following it. --><span id="arm_sve.h"></span> | |
`<arm_sve.h>` defines data types and intrinsics for SVE and its | ||
extensions; see [SVE language extensions and | ||
intrinsics](#sve-language-extensions-and-intrinsics) for details. | ||
You should test the `__ARM_FEATURE_SVE` macro before including the | ||
header: | ||
The `__ARM_FEATURE_SVE` macro can be tested before including the header. | ||
However, this is not recommended for cases where target feature selection is | ||
overridden within code, for example, when using #function-multi-versioning. | ||
|
||
``` c | ||
#ifdef __ARM_FEATURE_SVE | ||
|
@@ -1019,7 +1028,7 @@ Including `<arm_sve.h>` also includes the following header files: | |
|
||
`<arm_neon_sve_bridge.h>` defines intrinsics for moving data between | ||
Neon and SVE vector types; see [NEON-SVE Bridge](#neon-sve-bridge) | ||
for details. The `__ARM_NEON_SVE_BRIDGE` macro should be tested | ||
for details. The `__ARM_NEON_SVE_BRIDGE` macro should be tested | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: unnecessary change. |
||
before including the header: | ||
|
||
``` c | ||
|
@@ -1061,8 +1070,10 @@ change or be extended in the future. | |
|
||
`<arm_sme.h>` declares functions and defines intrinsics for SME | ||
and its extensions; see [SME language extensions and intrinsics](#sme-language-extensions-and-intrinsics) | ||
for details. The `__ARM_FEATURE_SME` macro should be tested before | ||
including the header: | ||
for details. The `__ARM_FEATURE_SME` macro can be tested before including the | ||
header. However, this is not recommended for cases where target feature | ||
selection is overridden within code, for example, when using | ||
#function-multi-versioning. | ||
|
||
``` c | ||
#ifdef __ARM_FEATURE_SME | ||
|
@@ -1072,6 +1083,34 @@ including the header: | |
|
||
Including `<arm_sme.h>` also includes [`<arm_sve.h>`](#arm_sve.h). | ||
|
||
### Predefined feature macros and header files | ||
|
||
CarolineConcatto marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Feature macros indicate whether that feature's intrinsics and inline assembly instructions | ||
are available in normal unannotated functions. In some implementations, the macro definitions | ||
are invariant throughout a translation unit, while in other implementations, the macro | ||
definitions can vary with pragmas. | ||
|
||
CarolineConcatto marked this conversation as resolved.
Show resolved
Hide resolved
|
||
The macro definitions are otherwise independent of where the test is performed, | ||
meaning that a feature macro does not guarantee that the feature can be used at the | ||
point that it is tested. For example: | ||
|
||
``` c | ||
#include <arm_sme.h> | ||
|
||
void foo(svbool_t pg, void *ptr, uint32_t slice_base) { | ||
#ifdef __ARM_FEATURE_SME | ||
svst1_hor_za8(0, slice_base, pg, ptr); | ||
CarolineConcatto marked this conversation as resolved.
Show resolved
Hide resolved
|
||
#endif | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought the paragraph above is more about the evaluation context/order of the macro to express that a user can't rely on the macro being set/unset when used with the ACLE keyword attributes, e.g.
The current example is more about showing that the compiler can add additional restrictions to the use of an intrinsic beyond what is captured by the ACLE macro. If that is the intent of the example, I think it would be helpful if that can be added as an separate paragraph, e.g. something like "The compiler may add additional restrictions to the intrinsics beyond what is captured by the ACLE macros depending on the context in which the intrinsics are used". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you @sdesmalen-arm for the suggestions. |
||
} | ||
``` | ||
|
||
While testing `__ARM_FEATURE_SME` ensures that the SME intrinsic `svst1_hor_za8` | ||
is available, `foo` will fail to compile because the call does not occur in a | ||
[streaming statement](#streaming-statement). | ||
CarolineConcatto marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Not all such issues can be caught during compilation and may instead result in | ||
runtime failures. | ||
CarolineConcatto marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
## Attributes | ||
|
||
GCC-style attributes are provided to annotate types, objects and | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the compiler sets the preprocessor macro to
1
, that indicates that it supports the ACLE for the specified feature and that the corresponding header file and intrinsics and types are available. The inverse is not necessarily true, i.e. when the compiler does not define the macro, that doesn't mean that the header file and corresponding intrinsics/types are not available. Compilers that support thetarget
attribute may (and probably should) make it possible to include these header files and make the intrinsics/types available, even when the macro is not set.However, when a user doesn't test for the macro, it could still be that the header file is not available and the compiler will fail to build the file. For example, if the compiler is too old to recognise
<arm_some_feature.h>
.It is probably sufficient to say that the header is guaranteed to be available if the macro is set. (same comment for all the similar cases below)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, but also:
__ARM_ACLE
is a special case in that, like__ARM_NEON_SVE_BRIDGE
, it describes the presence of a header file rather than a specific architecture feature. So I think the original text was ok too (and so is your suggestion).For the feature macros, maybe “is available in a normal unannotated function” would be more specific than “is available”.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this was suggested by @paulwalker-arm
The
XYZ
macro can be tested before including the header. However, this is not recommended for cases where target feature selection is overridden within code, for example, when using #function-multi-versioning.From another previous asked change.
I have no preference, but would be nice to agree in what to write here, before I change again. The sentence you see here is a mix of previous interaction that we agree we should replace taking into account FMV.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure that wording is appropriate for
__ARM_ACLE
though (specifically that macro, not others). There is no change in features that would affect whether the compiler provides<arm_acle.h>
. Intrinsics within<arm_acle.h>
that depend on features have their own__ARM_FEATURE_*
macro.