-
Notifications
You must be signed in to change notification settings - Fork 353
[FMV][AArch64] Set HAS_*_FMV macro and skip test if symbol is unavailable #269
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
base: main
Are you sure you want to change the base?
Conversation
Add conditional CFLAGS definitions for AArch64 targets to support FMV-related behavior on Darwin and Linux platforms. - Adds -DHAS_DARWIN_FMV when ARCH=AArch64 and TARGET_OS=Darwin - Adds -DHAS_LINUX_FMV when ARCH=AArch64 and TARGET_OS=Linux This change fixes build errors when running LNT tests on AArch64 systems with platform-specific FMV handling.
I am a bit puzzled with the equivalent cmake code
The way I read it |
@labrinea Thank you for your comment. I've confirmed how However, when I run LNT with a Makefile that includes my modifications, the build and execution complete without any issues. In other words, even though |
- Adds a pre-build check in the Makefile for the __init_cpu_features_resolver, __init_cpu_features symbol. - The result is used to define CHECK_FMV (HAS_DARWIN_FMV, HAS_LINUX_FMV), which conditionally compiles the platform-specific code in aarch64-init-cpu-features.c
@labrinea
To match the
TestingIn my local environment, the check for the Could you please test these changes in an environment where the |
I have raised #270 to address the problem. I am not sure who to ask for review. I tested it and works on a system that has the |
Hi @labrinea , Thank you for the review and for following up with a fix. I noticed that instead of merging PR #269, you made a separate commit to address the issue. If there are specific improvements or changes needed, I’d sincerely appreciate your guidance so that I can apply them myself. Thanks again for your time and help! |
Hi @IamYJLee. Apologies for not guiding you how to improve this PR instead. Since I spent the time investing this issue from my end I thought it's easier to raise a new PR with my changes. Allow me to explain why this patch doesn't work.
Feel free to adjust this PR accordingly and credit @momchil-velikov for helping me on this on the commit message. There are still two issues with my patch that need to be addressed, so we need someone to review these first. |
@labrinea I completely understand your rationale for submitting a new PR, and I appreciate the clarity you've provided regarding the behavior of With your permission, I would like to continue the work from here and revise the patch to address the remaining issues you mentioned. I will ensure that appropriate credit is given to you in the commit message for your valuable contributions. Please let me know if you have any objections. Otherwise, I’ll proceed with the necessary updates. |
This reverts commit ac0c956.
…able - Selects __init_cpu_features_resolver for Darwin, and __init_cpu_features for Linux. - Attempts to compile a dummy program to detect availability of the symbol via TARGET_LLVMGCC. - Adds corresponding macro (HAS_DARWIN_FMV or HAS_LINUX_FMV) if detection succeeds. - Skips test if the symbol is undefined. Credit to @momchil-velikov and @labrinea for their valuable input and support during review.
# TARGET_LLVMGCC refers to the compiler specified with the --cc option, | ||
# and can be checked in the build-tools.log file in the test results directory. | ||
SYMBOL := $(shell echo "extern void $(SYMBOL)(void); void (*p)() = $(SYMBOL); int main() {}" | \ | ||
$(TARGET_LLVMGCC) --rtlib=compiler-rt -x c - -o /dev/null 2>/dev/null && \ |
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.
TARGET_LLVMGCC
refers to the compiler specified with the --cc option, and can be checked in the build-tools.log
file in the test results directory.
@jroelofs @labrinea @momchil-velikov Could you please take a look? Thanks! |
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.
As I said earlier I am not sure if TARGET_LLVMGCC is the right compiler and whether --rtlib=compiler-rt should be inherited from some CFLAGS variable. I tested this patch and it worked in my environment.
# TARGET_LLVMGCC refers to the compiler specified with the --cc option, | ||
# and can be checked in the build-tools.log file in the test results directory. | ||
SYMBOL := $(shell echo "extern void $(SYMBOL)(void); void (*p)() = $(SYMBOL); int main() {}" | \ | ||
$(TARGET_LLVMGCC) --rtlib=compiler-rt -x c - -o /dev/null 2>/dev/null && \ |
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 reviewed the build/CMakeFiles/CMakeConfigureLog.yaml
to trace how check_symbol_exists
is executed through CMakeLists.txt
.
compiler-rt
(a runtime library) is not shown in the configuration log, but including it in the current patch appears to work fine without introducing any issues.
Here is check_symbol_exists
Log :
buildResult:
variable: "HAS_AARCH64_FMV"
cached: true
stdout: |
Change Dir: /Users/nshc-yjlee_m/Documents/dev/llvm-test-suite/build/CMakeFiles/CMakeScratch/TryCompile-j5irsv
Run Build Command(s):/opt/homebrew/bin/ninja -v cmTC_f539e && [1/2] /Applications/Xcode161_Testing.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc -arch arm64 -isysroot /Applications/Xcode161_Testing.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX15.1.sdk -MD -MT CMakeFiles/cmTC_f539e.dir/CheckSymbolExists.c.o -MF CMakeFiles/cmTC_f539e.dir/CheckSymbolExists.c.o.d -o CMakeFiles/cmTC_f539e.dir/CheckSymbolExists.c.o -c /Users/nshc-yjlee_m/Documents/dev/llvm-test-suite/build/CMakeFiles/CMakeScratch/TryCompile-j5irsv/CheckSymbolExists.c
[2/2] : && /Applications/Xcode161_Testing.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc -arch arm64 -isysroot /Applications/Xcode161_Testing.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX15.1.sdk -Wl,-search_paths_first -Wl,-headerpad_max_install_names CMakeFiles/cmTC_f539e.dir/CheckSymbolExists.c.o -o cmTC_f539e && :
exitCode: 0
The logs for |
ERROR CONTEXT
Before this patch, LNT test runs on AArch64 platforms (e.g., macOS arm64) could fail with missing symbol errors:
Patch Description
This change fixes build errors when running LNT tests on AArch64 systems with platform-specific FMV handling.
This patch reflects the changes from llvm/llvm-test-suite@d24d5dcd.
Credit to @momchil-velikov and @labrinea for their valuable input and support during review.