Skip to content

[ARM64EC] Fix compilation of intrin.h in ARM64EC mode. #87717

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

Merged
merged 2 commits into from
Apr 8, 2024
Merged

[ARM64EC] Fix compilation of intrin.h in ARM64EC mode. #87717

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f18163b
[ARM64EC] Fix compilation of intrin.h in ARM64EC mode.
efriedma-quic Apr 4, 2024
0436dc0
[fixup] add a few more __arm64ec__ checks.
efriedma-quic Apr 5, 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
17 changes: 9 additions & 8 deletions clang/lib/Headers/intrin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#include <intrin0.h>

/* First include the standard intrinsics. */
#if defined(__i386__) || defined(__x86_64__)
#if defined(__i386__) || (defined(__x86_64__) && !defined(__arm64ec__))
#include <x86intrin.h>
#endif

Expand Down Expand Up @@ -166,7 +166,7 @@ unsigned __int32 xbegin(void);
void _xend(void);

/* These additional intrinsics are turned on in x64/amd64/x86_64 mode. */
#ifdef __x86_64__
#if defined(__x86_64__) && !defined(__arm64ec__)
void __addgsbyte(unsigned long, unsigned char);
void __addgsdword(unsigned long, unsigned long);
void __addgsqword(unsigned long, unsigned __int64);
Expand Down Expand Up @@ -236,7 +236,8 @@ __int64 _mul128(__int64, __int64, __int64 *);
/*----------------------------------------------------------------------------*\
|* movs, stos
\*----------------------------------------------------------------------------*/
#if defined(__i386__) || defined(__x86_64__)

#if defined(__i386__) || (defined(__x86_64__) && !defined(__arm64ec__))
static __inline__ void __DEFAULT_FN_ATTRS __movsb(unsigned char *__dst,
unsigned char const *__src,
size_t __n) {
Expand Down Expand Up @@ -305,7 +306,7 @@ static __inline__ void __DEFAULT_FN_ATTRS __stosw(unsigned short *__dst,
: "memory");
}
#endif
#ifdef __x86_64__
#if defined(__x86_64__) && !defined(__arm64ec__)
static __inline__ void __DEFAULT_FN_ATTRS __movsq(
unsigned long long *__dst, unsigned long long const *__src, size_t __n) {
__asm__ __volatile__("rep movsq"
Expand All @@ -324,7 +325,7 @@ static __inline__ void __DEFAULT_FN_ATTRS __stosq(unsigned __int64 *__dst,
/*----------------------------------------------------------------------------*\
|* Misc
\*----------------------------------------------------------------------------*/
#if defined(__i386__) || defined(__x86_64__)
#if defined(__i386__) || (defined(__x86_64__) && !defined(__arm64ec__))
static __inline__ void __DEFAULT_FN_ATTRS __halt(void) {
__asm__ volatile("hlt");
}
Expand All @@ -339,7 +340,7 @@ static __inline__ void __DEFAULT_FN_ATTRS __nop(void) {
/*----------------------------------------------------------------------------*\
|* MS AArch64 specific
\*----------------------------------------------------------------------------*/
#if defined(__aarch64__)
#if defined(__aarch64__) || defined(__arm64ec__)
unsigned __int64 __getReg(int);
long _InterlockedAdd(long volatile *Addend, long Value);
__int64 _InterlockedAdd64(__int64 volatile *Addend, __int64 Value);
Expand Down Expand Up @@ -383,7 +384,7 @@ void __cdecl __prefetch(void *);
/*----------------------------------------------------------------------------*\
|* Privileged intrinsics
\*----------------------------------------------------------------------------*/
#if defined(__i386__) || defined(__x86_64__)
#if defined(__i386__) || (defined(__x86_64__) && !defined(__arm64ec__))
static __inline__ unsigned __int64 __DEFAULT_FN_ATTRS
__readmsr(unsigned long __register) {
// Loads the contents of a 64-bit model specific register (MSR) specified in
Expand All @@ -397,7 +398,6 @@ __readmsr(unsigned long __register) {
__asm__ ("rdmsr" : "=d"(__edx), "=a"(__eax) : "c"(__register));
return (((unsigned __int64)__edx) << 32) | (unsigned __int64)__eax;
}
#endif

static __inline__ unsigned __LPTRINT_TYPE__ __DEFAULT_FN_ATTRS __readcr3(void) {
unsigned __LPTRINT_TYPE__ __cr3_val;
Expand All @@ -413,6 +413,7 @@ static __inline__ void __DEFAULT_FN_ATTRS
__writecr3(unsigned __INTPTR_TYPE__ __cr3_val) {
__asm__ ("mov {%0, %%cr3|cr3, %0}" : : "r"(__cr3_val) : "memory");
}
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch. We should probably also ifdef these function declarations near the top of this file.

https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/intrin.h#L96
https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/intrin.h#L126

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That chunk of declarations near the top of the file needs a lot of work to match the actual implementation; I'd prefer to just leave it for now.


#ifdef __cplusplus
}
Expand Down
10 changes: 5 additions & 5 deletions clang/lib/Headers/intrin0.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
#ifndef __INTRIN0_H
#define __INTRIN0_H

#ifdef __x86_64__
#if defined(__x86_64__) && !defined(__arm64ec__)
#include <adcintrin.h>
#endif

Expand All @@ -27,7 +27,7 @@ unsigned char _BitScanForward(unsigned long *_Index, unsigned long _Mask);
unsigned char _BitScanReverse(unsigned long *_Index, unsigned long _Mask);
void _ReadWriteBarrier(void);

#if defined(__aarch64__)
#if defined(__aarch64__) || defined(__arm64ec__)
unsigned int _CountLeadingZeros(unsigned long);
unsigned int _CountLeadingZeros64(unsigned _int64);
unsigned char _InterlockedCompareExchange128_acq(__int64 volatile *_Destination,
Expand All @@ -44,7 +44,7 @@ unsigned char _InterlockedCompareExchange128_rel(__int64 volatile *_Destination,
__int64 *_ComparandResult);
#endif

#ifdef __x86_64__
#ifdef __x86_64__ && !defined(__arm64ec__)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hello, we found compile errors on this line:

(47,19): error: extra tokens at end of #ifdef directive [-Werror,-Wextra-tokens]

Seems like this line forgot to change #ifdef when changed into expression

Copy link
Contributor

Choose a reason for hiding this comment

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

@FreddyLeaf Curiosity how are you running clang. Since intrin0.h is treated as a system header warnings should be suppressed.

I couldn't repro the warning in my local testing due to intrin0.h being found via a system include path.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I don't have the reproducer, either. The scene supposed to be building V-Ray with MSbuild on Windows.

unsigned __int64 _umul128(unsigned __int64, unsigned __int64,
unsigned __int64 *);
unsigned __int64 __shiftleft128(unsigned __int64 _LowPart,
Expand All @@ -55,7 +55,7 @@ unsigned __int64 __shiftright128(unsigned __int64 _LowPart,
unsigned char _Shift);
#endif

#if defined(__x86_64__) || defined(__i386__)
#if defined(__i386__) || (defined(__x86_64__) && !defined(__arm64ec__))
void _mm_pause(void);
#endif

Expand Down Expand Up @@ -83,7 +83,7 @@ __int64 _InterlockedXor64(__int64 volatile *_Value, __int64 _Mask);
__int64 _InterlockedAnd64(__int64 volatile *_Value, __int64 _Mask);
#endif

#if defined(__arm__) || defined(__aarch64__)
#if defined(__arm__) || defined(__aarch64__) || defined(__arm64ec__)
/*----------------------------------------------------------------------------*\
|* Interlocked Exchange Add
\*----------------------------------------------------------------------------*/
Expand Down
14 changes: 12 additions & 2 deletions clang/test/Headers/ms-intrin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,16 @@
// RUN: -ffreestanding -fsyntax-only -Werror \
// RUN: -isystem %S/Inputs/include %s

// RUN: %clang_cc1 -triple aarch64--windows \
// RUN: -fms-compatibility -fms-compatibility-version=17.00 \
// RUN: -ffreestanding -fsyntax-only -Werror \
// RUN: -isystem %S/Inputs/include %s

// RUN: %clang_cc1 -triple arm64ec--windows \
// RUN: -fms-compatibility -fms-compatibility-version=17.00 \
// RUN: -ffreestanding -fsyntax-only -Werror \
// RUN: -isystem %S/Inputs/include %s

// REQUIRES: x86-registered-target

// intrin.h needs size_t, but -ffreestanding prevents us from getting it from
Expand All @@ -41,15 +51,15 @@ void f() {
__stosd(0, 0, 0);
__stosw(0, 0, 0);

#ifdef _M_X64
#if defined(_M_X64) && !defined(_M_ARM64EC)
__movsq(0, 0, 0);
__stosq(0, 0, 0);
#endif

int info[4];
__cpuid(info, 0);
__cpuidex(info, 0, 0);
#if defined(_M_X64) || defined(_M_IX86)
#if (defined(_M_X64) && !defined(_M_ARM64EC)) || defined(_M_IX86)
_xgetbv(0);
#endif
__halt();
Expand Down