Skip to content
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

[Premerge] Add flang-rt #128678

Merged
merged 20 commits into from
Mar 13, 2025
Merged

[Premerge] Add flang-rt #128678

Changes from 3 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
9ebeeae
[Premerge] Add flang-rt
Meinersbur Feb 25, 2025
afada27
Don't change deps
Meinersbur Feb 28, 2025
8345c19
flang-rt has dependencies
Meinersbur Feb 28, 2025
da2a539
flang-rt is a runtime
Meinersbur Mar 3, 2025
93a7713
Merge branch 'flang_runtime_premerge' into users/meinersbur/flang_run…
Meinersbur Mar 3, 2025
a8a9323
Fix syntax error
Meinersbur Mar 4, 2025
a7b728c
flang_runtime_amdgpu-offload
Meinersbur Mar 4, 2025
b0d0879
Pass Flang as Fortran compiler
Meinersbur Mar 5, 2025
8c41ae6
Merge branch 'main' into users/meinersbur/flang_runtime_premerge
Meinersbur Mar 5, 2025
e8b028a
Minimal changes
Meinersbur Mar 5, 2025
67809ce
Add change in flang/ to trigger pre-merge check
Meinersbur Mar 5, 2025
d2144a0
Undo unrelated change
Meinersbur Mar 5, 2025
26bb93a
Set FLANG_ENABLE_FLANG_RT=OFF
Meinersbur Mar 5, 2025
6e7d7fd
Pass LLVM_BINARY_DIR required for testing
Meinersbur Mar 5, 2025
bed0c3f
Adjust all runtime build configurations
Meinersbur Mar 5, 2025
59ec1ee
Trigger build of Flang when flang-rt is changed
Meinersbur Mar 6, 2025
637f662
Test change in flang-rt to trigger flang compilation
Meinersbur Mar 6, 2025
881b6bf
Consider changes in flang-rt
Meinersbur Mar 6, 2025
3d95f2e
Exclude flang-rt from LLVM_ENABLE_PROJECTS
Meinersbur Mar 6, 2025
b2354ff
Do not add flang-rt to projects-to-test in the first place
Meinersbur Mar 6, 2025
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
14 changes: 13 additions & 1 deletion .ci/compute-projects.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ function compute-projects-to-test() {
# Flang is not stable in Windows CI at the moment
if [[ $isForWindows == 0 ]]; then
echo flang
echo flang-rt
fi
;;
clang)
Expand All @@ -47,6 +48,7 @@ function compute-projects-to-test() {
# Flang is not stable in Windows CI at the moment
if [[ $isForWindows == 0 ]]; then
echo flang
echo flang-rt
Copy link
Contributor

Choose a reason for hiding this comment

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

Putting it here will still build it using the LLVM_ENABLE_PROJECTS mechanism. Is that intentional?

Copy link
Member Author

Choose a reason for hiding this comment

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

No it is not. I think I assumed that it worked like zorg's depends_on_projects.

But now that I look closer to it, I am even more puzzled. If there is a file changed in a runtime, let's say libcxx, it will not be added to linux_projects_to_test, and hence nothing is added to linux_runtimes_to_test? Am I missing something? Why does all_projects list also the runtimes?

Copy link
Contributor

Choose a reason for hiding this comment

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

They're all built as projects currently. It's on my TODO list to migrate them, but I haven't gotten around to it yet.

As far as I understand libcxx;libcxxabi;libunwind are the three that go through the runtimes build.

If everything works like this, I'm perfectly fine with merging and then changing it around when I switch how the runtimes build works in the premerge system. Otherwise, having it be implicit added when building flang is fine with me, and we can revisit when I switch stuff over to the proper runtimes build.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added flang-rt to compute-runtimes-to-test instead. Please also review the changes in add-dependencies, I think the entry on flang-rt is useless then.

Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tested this locally? I don't think this will work as is. On Linux the runtimes explicitly built through the runtimes build are in

llvm-project/.ci/monolithic-linux.sh

Line 95 in 6eefadd

cmake -S "${MONOREPO_ROOT}/runtimes" -B "${RUNTIMES_BUILD_DIR}" -GNinja \

I would think the path to Flang would need to be passed in there? No idea how the plumbing works for flang-rt though. The runtimes are also built three separate times in different C++ configurations, which doesn't make sense for a library written in Fortran, although looking at the source, it seems like most of it is in C++?

If this isn't a necessary change for keeping the flang build working, it might be worth just waiting until I've had a chance to rework how we test runtimes in premerge.

Copy link
Member Author

Choose a reason for hiding this comment

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

You are correct, the path to Flang must be passed to the runtimes build. Thanks for noticing.

I have not tested it locally yet. There are some reasons for that

  1. I was assuming that the premerge-monolithic-linux builder (which I tested, see LLVM_ENABLE_RUNTIMES=flang-rt for premerge-monolithic-* llvm-zorg#396) was resembling the premerge configuration (which is a bootstrapping build), as was alluded to by a comment.
  2. I don't have a local BuildKite infrastructure
  3. I was hoping that the pre-merge check of this PR was testing it. While it was in draft mode, it did not show errors.
  4. The configuration makes use of a Google-proprietary extensions, such as "-gmlt"

I will try to make it run locally anyway.

fi
;;
*)
Expand Down Expand Up @@ -95,11 +97,21 @@ function add-dependencies() {
compiler-rt|libc|openmp)
echo clang lld
;;
flang|lldb|libclc)
lldb|libclc)
for p in llvm clang; do
echo $p
done
;;
flang)
for p in llvm clang mlir; do
echo $p
done
;;
flang-rt)
for p in llvm clang mlir flang; do
echo $p
done
;;
lld|mlir|polly)
echo llvm
;;
Expand Down