-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[compiler-rt] Use mangled function names on ARM64EC #137960
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
Conversation
On ARM64EC, function names and calls (but not address-taking or data symbol references) use symbols prefixed with "#". Since it's an unique behavior, introduce a new FUNC_SYMBOL macro instead of reusing something like SYMBOL_NAME, which is also used for data symbols. Based on patch by Billy Laws.
|
CC @bylaws |
You can test this locally with the following command:git-clang-format --diff HEAD~1 HEAD --extensions h -- compiler-rt/lib/builtins/assembly.hView the diff from clang-format here.diff --git a/compiler-rt/lib/builtins/assembly.h b/compiler-rt/lib/builtins/assembly.h
index 385dc1903..37ad00258 100644
--- a/compiler-rt/lib/builtins/assembly.h
+++ b/compiler-rt/lib/builtins/assembly.h
@@ -61,10 +61,7 @@
#define LOCAL_LABEL(name) .L ## name
#define FILE_LEVEL_DIRECTIVE
#define SYMBOL_IS_FUNC(name) \
- .def FUNC_SYMBOL(name) SEPARATOR \
- .scl 2 SEPARATOR \
- .type 32 SEPARATOR \
- .endef
+ .def FUNC_SYMBOL(name) SEPARATOR.scl 2 SEPARATOR.type 32 SEPARATOR.endef
#define CONST_SECTION .section .rdata,"rd"
#define NO_EXEC_STACK_DIRECTIVE
@@ -231,55 +228,52 @@
#define DEFINE_COMPILERRT_FUNCTION(name) \
DEFINE_CODE_STATE \
- FILE_LEVEL_DIRECTIVE SEPARATOR \
- .globl FUNC_SYMBOL(SYMBOL_NAME(name)) SEPARATOR \
- SYMBOL_IS_FUNC(SYMBOL_NAME(name)) SEPARATOR \
- DECLARE_SYMBOL_VISIBILITY(name) \
+ FILE_LEVEL_DIRECTIVE SEPARATOR.globl FUNC_SYMBOL(SYMBOL_NAME(name)) \
+ SEPARATOR SYMBOL_IS_FUNC(SYMBOL_NAME(name)) \
+ SEPARATOR DECLARE_SYMBOL_VISIBILITY(name) \
DECLARE_FUNC_ENCODING \
- FUNC_SYMBOL(SYMBOL_NAME(name)):
+ FUNC_SYMBOL(SYMBOL_NAME(name)) :
#define DEFINE_COMPILERRT_THUMB_FUNCTION(name) \
DEFINE_CODE_STATE \
FILE_LEVEL_DIRECTIVE SEPARATOR \
- .globl FUNC_SYMBOL(SYMBOL_NAME(name)) SEPARATOR \
- SYMBOL_IS_FUNC(SYMBOL_NAME(name)) SEPARATOR \
- DECLARE_SYMBOL_VISIBILITY(name) SEPARATOR \
- .thumb_func SEPARATOR \
- FUNC_SYMBOL(SYMBOL_NAME(name)):
+ .globl \
+ FUNC_SYMBOL(SYMBOL_NAME(name)) SEPARATOR SYMBOL_IS_FUNC( \
+ SYMBOL_NAME(name)) SEPARATOR DECLARE_SYMBOL_VISIBILITY(name) \
+ SEPARATOR.thumb_func SEPARATOR FUNC_SYMBOL(SYMBOL_NAME(name)) \
+ :
#define DEFINE_COMPILERRT_PRIVATE_FUNCTION(name) \
DEFINE_CODE_STATE \
- FILE_LEVEL_DIRECTIVE SEPARATOR \
- .globl FUNC_SYMBOL(SYMBOL_NAME(name)) SEPARATOR \
- SYMBOL_IS_FUNC(SYMBOL_NAME(name)) SEPARATOR \
- HIDDEN(SYMBOL_NAME(name)) SEPARATOR \
- DECLARE_FUNC_ENCODING \
- FUNC_SYMBOL(SYMBOL_NAME(name)):
+ FILE_LEVEL_DIRECTIVE SEPARATOR.globl FUNC_SYMBOL(SYMBOL_NAME(name)) \
+ SEPARATOR SYMBOL_IS_FUNC(SYMBOL_NAME(name)) \
+ SEPARATOR HIDDEN(SYMBOL_NAME(name)) \
+ SEPARATOR DECLARE_FUNC_ENCODING FUNC_SYMBOL(SYMBOL_NAME(name)) \
+ :
#define DEFINE_COMPILERRT_PRIVATE_FUNCTION_UNMANGLED(name) \
- DEFINE_CODE_STATE \
- .globl FUNC_SYMBOL(name) SEPARATOR \
- SYMBOL_IS_FUNC(name) SEPARATOR \
- HIDDEN(name) SEPARATOR \
+ DEFINE_CODE_STATE.globl FUNC_SYMBOL(name) \
+ SEPARATOR \
+ SYMBOL_IS_FUNC(name) SEPARATOR HIDDEN(name) \
+ SEPARATOR \
DECLARE_FUNC_ENCODING \
- FUNC_SYMBOL(name):
+ FUNC_SYMBOL(name) :
#define DEFINE_COMPILERRT_OUTLINE_FUNCTION_UNMANGLED(name) \
DEFINE_CODE_STATE \
- FUNC_ALIGN \
- .globl FUNC_SYMBOL(name) SEPARATOR \
- SYMBOL_IS_FUNC(name) SEPARATOR \
- DECLARE_SYMBOL_VISIBILITY_UNMANGLED(FUNC_SYMBOL(name)) SEPARATOR \
- DECLARE_FUNC_ENCODING \
- FUNC_SYMBOL(name): \
- SEPARATOR CFI_START \
- SEPARATOR BTI_C
+ FUNC_ALIGN.globl FUNC_SYMBOL(name) \
+ SEPARATOR \
+ SYMBOL_IS_FUNC(name) \
+ SEPARATOR DECLARE_SYMBOL_VISIBILITY_UNMANGLED(FUNC_SYMBOL(name)) \
+ SEPARATOR DECLARE_FUNC_ENCODING FUNC_SYMBOL(name) \
+ : SEPARATOR CFI_START SEPARATOR BTI_C
#define DEFINE_COMPILERRT_FUNCTION_ALIAS(name, target) \
- .globl FUNC_SYMBOL(SYMBOL_NAME(name)) SEPARATOR \
- SYMBOL_IS_FUNC(SYMBOL_NAME(name)) SEPARATOR \
- DECLARE_SYMBOL_VISIBILITY(name) SEPARATOR \
- .set FUNC_SYMBOL(SYMBOL_NAME(name)), FUNC_SYMBOL(target) SEPARATOR
+ .globl FUNC_SYMBOL(SYMBOL_NAME(name)) \
+ SEPARATOR SYMBOL_IS_FUNC(SYMBOL_NAME(name)) \
+ SEPARATOR DECLARE_SYMBOL_VISIBILITY(name) \
+ SEPARATOR.set FUNC_SYMBOL(SYMBOL_NAME(name)), \
+ FUNC_SYMBOL(target) SEPARATOR
#if defined(__ARM_EABI__)
#define DEFINE_AEABI_FUNCTION_ALIAS(aeabi_name, name) \
|
|
FWIW, the formatting warnings are from clang-format trying to adjust indentation in modified macros. I believe the current formatting is intentional and is meant to remain unchanged. |
mstorsjo
left a comment
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.
Looks good to me overall.
|
I added a couple other reviewers as well, in case they have comments on it, but this does look good to me - and in case the others have comments within a couple of days, we can probably merge this. |
compnerd
left a comment
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 wonder if there is a way to unify SYMBOL_IS_FUNC and FUNC_SYMBOL. We have a number of different places where we indicate that a label is a function, and it seems it would be very confusing for most to know which one belongs where.
Thanks for the reviews. The best I could come up with is renaming |
... during a refactoring for arm64ec in llvm#137960 6ade80c. The subtle change here results in a change in symbols in the library from: ``` 00000038 T ___gesf2 00000038 T ___gtsf2 ``` to: ``` I ___gesf2 (indirect for __gtsf2) 00000038 T ___gtsf2 ``` ... which is a breaking change on Mach-O platforms. rdar://157846316
…IAS (#156979) ... during a refactoring for arm64ec in #137960 6ade80c. The subtle change here results in a change in symbols in the library from: ``` 00000038 T ___gesf2 00000038 T ___gtsf2 ``` to: ``` I ___gesf2 (indirect for __gtsf2) 00000038 T ___gtsf2 ``` ... which is a breaking change on Mach-O platforms. rdar://157846316
On ARM64EC, function names and calls (but not address-taking or data symbol references) use symbols prefixed with "#". Since it's an unique behavior, introduce a new
FUNC_SYMBOLmacro instead of reusing something likeSYMBOL_NAME, which is also used for data symbols.Based on patch by Billy Laws.