Clang 17 and later rejects __attribute__ after static specifier in C++23 lambda expressions

[johnplatts]

Here is a snippet of C++23 code that compiles successfully with GCC 13 or later with -std=c++23 but fails to compile with Clang 17 or later (even with the std=c++23 option):

#include <stddef.h>
#include <stdint.h>
#include <immintrin.h>

__m128i __attribute__((__target__("sse4.2"))) SomeFunc(
    const int32_t* src_ptr, size_t num_of_vectors) {
    auto process_func = [](const int32_t* p) static __attribute__((__target__("sse4.2"))) {
        __m128i v = _mm_loadu_si128(reinterpret_cast<const __m128i*>(p));
        return _mm_mullo_epi32(v, _mm_set_epi32(15752961, 3969, 63, 1));
    };

    __m128i h = _mm_setzero_si128();
    for (size_t i = 0; i < num_of_vectors; i++) {
        h = _mm_add_epi32(_mm_sub_epi32(_mm_slli_epi32(h, 6), h),
            process_func(src_ptr + 4 * i));
    }
    return h;
}

The results of compiling the above snippet with GCC 13, Clang 17, and Clang trunk can be found at https://godbolt.org/z/WfoqnnPqc.

Should Clang be updated to allow __attribute__((__target__("sse4.2"))) after the static specifier in C++23 lambda expressions?

On the other hand, the below snippet successfully compiles with Clang 17 or later in C++23 mode but fails to compile with GCC 13 in C++23 mode:

#include <stddef.h>
#include <stdint.h>
#include <immintrin.h>

__m128i __attribute__((__target__("sse4.2"))) SomeFunc(
    const int32_t* src_ptr, size_t num_of_vectors) {
    auto process_func = [](const int32_t* p) __attribute__((__target__("sse4.2"))) static {
        __m128i v = _mm_loadu_si128(reinterpret_cast<const __m128i*>(p));
        return _mm_mullo_epi32(v, _mm_set_epi32(15752961, 3969, 63, 1));
    };

    __m128i h = _mm_setzero_si128();
    for (size_t i = 0; i < num_of_vectors; i++) {
        h = _mm_add_epi32(_mm_sub_epi32(_mm_slli_epi32(h, 6), h),
            process_func(src_ptr + 4 * i));
    }
    return h;
}

The results of compiling the above snippet with GCC 13, Clang 17, and Clang trunk can be found at https://godbolt.org/z/9ooT8bj1f.

[cor3ntin]

@AaronBallman @erichkeane

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: John Platts (johnplatts)

Here is a snippet of C++23 code that compiles successfully with GCC 13 or later with `-std=c++23` but fails to compile with Clang 17 or later (even with the `std=c++23` option): ``` #include <stddef.h> #include <stdint.h> #include <immintrin.h>

__m128i attribute((target("sse4.2"))) SomeFunc(
const int32_t* src_ptr, size_t num_of_vectors) {
auto process_func = [](const int32_t* p) static attribute((target("sse4.2"))) {
__m128i v = _mm_loadu_si128(reinterpret_cast<const __m128i*>(p));
return _mm_mullo_epi32(v, _mm_set_epi32(15752961, 3969, 63, 1));
};

__m128i h = _mm_setzero_si128();
for (size_t i = 0; i < num_of_vectors; i++) {
    h = _mm_add_epi32(_mm_sub_epi32(_mm_slli_epi32(h, 6), h),
        process_func(src_ptr + 4 * i));
}
return h;

}

The results of compiling the above snippet with GCC 13, Clang 17, and Clang trunk can be found at https://godbolt.org/z/WfoqnnPqc.

Should Clang be updated to allow `__attribute__((__target__("sse4.2")))` after the `static` specifier in C++23 lambda expressions?

On the other hand, the below snippet successfully compiles with Clang 17 or later in C++23 mode but fails to compile with GCC 13 in C++23 mode:

#include <stddef.h>
#include <stdint.h>
#include <immintrin.h>

__m128i attribute((target("sse4.2"))) SomeFunc(
const int32_t* src_ptr, size_t num_of_vectors) {
auto process_func = [](const int32_t* p) attribute((target("sse4.2"))) static {
__m128i v = _mm_loadu_si128(reinterpret_cast<const __m128i*>(p));
return _mm_mullo_epi32(v, _mm_set_epi32(15752961, 3969, 63, 1));
};

__m128i h = _mm_setzero_si128();
for (size_t i = 0; i < num_of_vectors; i++) {
    h = _mm_add_epi32(_mm_sub_epi32(_mm_slli_epi32(h, 6), h),
        process_func(src_ptr + 4 * i));
}
return h;

}

The results of compiling the above snippet with GCC 13, Clang 17, and Clang trunk can be found at https://godbolt.org/z/9ooT8bj1f.
</details>

[erichkeane]

Huh, neat! It looks like we just disagree about the location for parsing the attributes. Annoyingly, we have parsed in that location for a while, so we probably have to parse in BOTH locations in order to be both clang backward, and gcc compatible. Likely just a missing "MaybeParseGCCAttributes" line somewhere. We might want to do an analysis as a part of this for what MSVC does in these areas.

[johnplatts]

Huh, neat! It looks like we just disagree about the location for parsing the attributes. Annoyingly, we have parsed in that location for a while, so we probably have to parse in BOTH locations in order to be both clang backward, and gcc compatible. Likely just a missing "MaybeParseGCCAttributes" line somewhere. We might want to do an analysis as a part of this for what MSVC does in these areas.

MSVC does not currently support the static lambda specifier (even with the -std:c++latest compiler option), but here is a snippet with the [[msvc::forceinline]] attribute declared after the noexcept lambda specifier that does compile with MSVC with the -std:c++latest option:

#include <stddef.h>
#include <stdint.h>
#include <immintrin.h>

__m128i SomeFunc(const int32_t* src_ptr, size_t num_of_vectors) {
    auto process_func = [](const int32_t* p) noexcept [[msvc::forceinline]] {
        __m128i v = _mm_loadu_si128(reinterpret_cast<const __m128i*>(p));
        return _mm_mullo_epi32(v, _mm_set_epi32(15752961, 3969, 63, 1));
    };

    __m128i h = _mm_setzero_si128();
    for (size_t i = 0; i < num_of_vectors; i++) {
        h = _mm_add_epi32(_mm_sub_epi32(_mm_slli_epi32(h, 6), h),
            process_func(src_ptr + 4 * i));
    }
    return h;
}

[cor3ntin]

@AaronBallman @erichkeane @DenisGZM is that fixed by #133107 ?

[DenisGZM]

@AaronBallman @erichkeane @DenisGZM is that fixed by #133107 ?

Unfortunately, no. This problem has another origin:

In clang/lib/Parse/ParseExprCXX.cpp:1520

if (HasParentheses || HasSpecifiers) {
    // GNU-style attributes must be parsed before the mutable specifier to
    // be compatible with GCC. MSVC-style attributes must be parsed before
    // the mutable specifier to be compatible with MSVC.
    MaybeParseAttributes(PAKM_GNU | PAKM_Declspec, Attributes);
   ...

we literally force us to parse attributes right before lambda-specifier (static/mutable etc.) which isn't seem right, at least now

[safocl]

_mm_loadu_si128(reinterpret_cast<const __m128i*>(p));

is this correct cast? not UB (from int32* to __m128i*)?

[johnplatts]

_mm_loadu_si128(reinterpret_cast<const __m128i*>(p));

is this correct cast? not UB (from int32* to __m128i*)?

Clang actually implements the _mm_loadu_si128 intrinsic by casting the pointer to a const struct __loadu_si128 *, which is safe as struct __loadu_si128 has the __may_alias__ and __packed__ attributes that allow const struct __loadu_si128* to alias any valid memory location that has at least 16 accessible bytes.

Likewise, Clang implements the _mm_storeu_si128 intrinsic by casting the pointer to a struct __storeu_si128*, which is safe as struct __storeu_si128 has the __may_alias__ and __packed__ attributes that allow struct __storeu_si128* to alias any valid memory location that has at least 16 writable bytes.

The cast to const __m128i* is correct here as Clang's implementations of _mm_loadu_si128 and _mm_storeu_si128 actually cast this pointer to a type that has the __may_alias__ attribute (which can be found at https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/emmintrin.h), avoiding issues with strict aliasing when compiling with Clang.