Skip to content
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

Jump to bottom

Remove request for preprocessor guards from header files. #321

Merged
merged 14 commits into from
Aug 14, 2024
Merged

Remove request for preprocessor guards from header files. #321

Changes from 7 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
fc50eb1
Remove request for preprocessor guards from header files.
CarolineConcatto May 29, 2024
994aec6
Re-add the need for __STDC_FORMAT_MACROS
CarolineConcatto May 30, 2024
1270932
Fix unwanted change
CarolineConcatto May 30, 2024
6e3991a
Re-add __ARM_NEON_SVE_BRIDGE it is related to compiler support
CarolineConcatto May 30, 2024
9079875
Adde missing sentence for __ARM_NEON_SVE_BRIDGE
CarolineConcatto May 30, 2024
ae42458
Update acle.md
CarolineConcatto May 30, 2024
92e5b8c
Add more information about the macro headers
CarolineConcatto Jul 2, 2024
fb4b4f8
Add a section in headers to explain headers and macros
CarolineConcatto Jul 8, 2024
15bd063
Re-write section with Predefined macros and header files
CarolineConcatto Aug 2, 2024
4fafea9
Apply suggestions from code review
CarolineConcatto Aug 6, 2024
50f5670
Re-write predefined feature macros and header files section
CarolineConcatto Aug 6, 2024
9d357be
Apply suggestions from code review
CarolineConcatto Aug 7, 2024
cd750bf
Address review comments
CarolineConcatto Aug 7, 2024
7b3ee34
Re-write sentence about the header availability
CarolineConcatto Aug 7, 2024
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
55 changes: 42 additions & 13 deletions main/acle.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -925,8 +925,8 @@ 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, but this might limit function multi-versioning capabilities:
Copy link

Choose a reason for hiding this comment

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

Ignore me if this format has already been agreed but if not can I recommend:

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.


``` c
#ifdef __ARM_ACLE
Expand All @@ -939,8 +939,9 @@ 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, but this might limit function
multi-versioning capabilities:

``` c
#ifdef __ARM_FEATURE_FP16_SCALAR_ARITHMETIC
Expand All @@ -953,8 +954,8 @@ 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, but this might limit function multi-versioning capabilities:

``` c
#ifdef __ARM_FEATURE_BF16
Expand All @@ -975,8 +976,9 @@ 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, but this might limit function multi-versioning
capabilities:

``` c
#ifdef __ARM_NEON
Expand All @@ -999,8 +1001,8 @@ 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,
but this might limit function multi-versioning capabilities:

``` c
#ifdef __ARM_FEATURE_SVE
Expand All @@ -1019,7 +1021,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
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: unnecessary change.

before including the header:

``` c
Expand Down Expand Up @@ -1061,8 +1063,8 @@ 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, but this might limit function multi-versioning capabilities:

``` c
#ifdef __ARM_FEATURE_SME
Expand All @@ -1072,6 +1074,33 @@ including the header:

Including `<arm_sme.h>` also includes [`<arm_sve.h>`](#arm_sve.h).

These macro features:
Copy link

Choose a reason for hiding this comment

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

This paragraph is added in a strange place, to me looks like it will fall int the section about arm_sme, while this is not strictly related to sme, maybe better to move it to the beginning, I mean the paragraphs about "Header files"?
IMO you can be more general and do not have to list all of the feature macros.

`__ARM_ACLE`, `__ARM_FEATURE_FP16_SCALAR_ARITHMETIC`, `__ARM_FEATURE_BF16`,
`__ARM_NEON`, `__ARM_FEATURE_SVE` and ` __ARM_FEATURE_SME`
represent a compiler's ability to use the instructions associated
with the feature via ACLE documented builtins and within inline assembly.
Copy link

Choose a reason for hiding this comment

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

I think it is better to use "documented intrinsics", the fact that they are implemented as builtins is an implantation detail.

They may also be influenced by source code mechanisms like `#pragma` or
`attribute`, but users should not rely on this. They are defined for user
Copy link

Choose a reason for hiding this comment

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

this is badly phrased. Do you mean here that users should not rely on #pragma or attribute or the feature.
macros?

If it is about the macros, I think we need to phrase it differently and explain that the presence of the macro does not guarantee that the architecture feature is always present at various stages of compilation or execution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Maciej,
I re-wrote this paragraph and added a section at the end of HEADER to explain headers + macros.
Just in case the ACLE has a section: Feature test macros. It is generic, it is about all predefined macros.
The point here is that the ACLE/I want to explain that headers protected by macro does not imply we can always use intrinsics.

convenience.

CarolineConcatto marked this conversation as resolved.
Show resolved Hide resolved
CarolineConcatto marked this conversation as resolved.
Show resolved Hide resolved
For example:

``` c
#ifdef __ARM_FEATURE_SME
#include<arm_sme.h>
#endif
void foo(svbool_t pg, void *ptr, uint32_t slice_base) {
svst1_hor_za8(0, slice_base, pg, ptr);
CarolineConcatto marked this conversation as resolved.
Show resolved Hide resolved
}
```

`foo` uses `__ARM_FEATURE_SME` to have available SME intriniscs,
like `svst1_hor_za8`. However it has the compilation error:
`builtin can only be called from a streaming function`
for the command line:
`armclang -O3 -target aarch64-linux-gnu -march=armv8-a+sme`
that defines `__ARM_FEATURE_SME` to one.

## Attributes

GCC-style attributes are provided to annotate types, objects and
Expand Down
Loading