-
Notifications
You must be signed in to change notification settings - Fork 77
[Flang-RT] Changes required for embedded GPU LLVM IR Flang runtime #328
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
[Flang-RT] Changes required for embedded GPU LLVM IR Flang runtime #328
Conversation
|
!PSDB |
|
Looks like there are smoke-fort failures in the PSDB related to stop/abort emissary support [2025-10-21T17:55:32.127Z] flang-gpu-abort: Make Failed |
| def : Flag<["-"], "nocudalib">, Alias<no_offloadlib>; | ||
| def gpulibc : Flag<["-"], "gpulibc">, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>, | ||
| HelpText<"Link the LLVM C Library for GPUs">; | ||
| def nogpuflangrt : Flag<["-"], "nogpuflangrt">, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>; |
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.
could we use the -f form of the flag ? and have both
-fno-gpuflangrt and -fgpurflangrt ?
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.
done
flang-rt/CMakeLists.txt
Outdated
| option(FLANG_RT_ENABLE_STATIC "Build Flang-RT as a static library." ON) | ||
| option(FLANG_RT_EMBED_GPU_LLVM_IR "Build Flang-RT as GPU LLVM IR library" ON) | ||
| if (FLANG_RT_EMBED_GPU_LLVM_IR) | ||
| add_compile_definitions(EMBED_FLANG_RT_GPU_LLVM_IR) |
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.
Prefer 'target_compile_defintions' in 'add_flangrt_library' which is preferred in "modern CMake" style because it avoids global changes.
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.
done
Meinersbur
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.
LGTM
| target_link_libraries(PluginCommon PRIVATE flang_rt.runtime | ||
| -L${CMAKE_BINARY_DIR}/../../lib -L${CMAKE_INSTALL_PREFIX}/lib) | ||
| endif() | ||
|
|
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.
[nit] please keep the diff to upstream as small as possible
| if (LLVM_INCLUDE_EXAMPLES) | ||
| add_subdirectory(examples) | ||
| endif () | ||
|
|
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.
[nit] please keep the diff to upstream as small as possible
23ee69b to
d00044d
Compare
|
Patch merged as: #757 . |
Motivation
Reduce linking time by embedding GPU LLVM IR code for Flang runtime.
Technical Details
Modified Flang runtime (added ifdefs to eliminate host-specific code from GPU compilation).
Test Plan
Checked smoke-fort-dev tests with new GPU Flang RT and the old flang_rt.hostdevice.
Test Result
I was able to compile and successfully run tests without flang_rt.hostdevice flag together with: ROCm/aomp#1595