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.

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

Enable the llvm offload build configuration #131527

Merged
merged 1 commit into from
Oct 26, 2024

Conversation

ZuseZ4
Copy link
Contributor

@ZuseZ4 ZuseZ4 commented Oct 11, 2024

@rustbot
Copy link
Collaborator

rustbot commented Oct 11, 2024

r? @Kobzol

rustbot has assigned @Kobzol.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Oct 11, 2024
@ZuseZ4
Copy link
Contributor Author

ZuseZ4 commented Oct 11, 2024

argh, I thought the draft state would prevent bors from rolling.
r? @ghost

@rustbot
Copy link
Collaborator

rustbot commented Oct 11, 2024

Failed to set assignee to ghost: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

@jieyouxu jieyouxu added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 11, 2024
@ZuseZ4 ZuseZ4 force-pushed the enable-llvm-offload-runtime branch from c1679ba to 36c458d Compare October 11, 2024 21:27
@ZuseZ4 ZuseZ4 changed the title WIP - enable the llvm offload build configuration Enable the llvm offload build configuration Oct 11, 2024
@ZuseZ4
Copy link
Contributor Author

ZuseZ4 commented Oct 11, 2024

r? @Kobzol

Generally, offload is the llvm feature to run code on the GPU and part of my project goal, so this is just the first PR to get warm. Do you want any changes?

This implicitly also enables openmp, since LLVM offload is a feature which developed out of openmp.
It's in some ongoing refactoring so offload is going to work without building openmp in the future, but the build is going to fail of offload is enabled without openmp. I don't think there is a good reason to build rustc with openmp but without offload, so I only have one flag, which people can't get wrong. Are you fine with that?

Edit: updated to include a src/bootstrap/src/utils/change_tracker.rs update.

@ZuseZ4 ZuseZ4 marked this pull request as ready for review October 11, 2024 22:13
@rustbot
Copy link
Collaborator

rustbot commented Oct 11, 2024

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

This PR modifies config.example.toml.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp.

@ZuseZ4 ZuseZ4 force-pushed the enable-llvm-offload-runtime branch from 36c458d to b1f21b8 Compare October 11, 2024 22:16
@@ -84,6 +84,9 @@
# Wheter to build Enzyme as AutoDiff backend.
#enzyme = false

# Whether to build LLVM with support for it's gpu offload runtime.
#offload = false
Copy link
Member

Choose a reason for hiding this comment

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

Maybe call it gpu-offload? Or does it also support offloading to other hardware like programmable network controllers and flash drives?

Copy link
Contributor Author

@ZuseZ4 ZuseZ4 Oct 12, 2024

Choose a reason for hiding this comment

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

only compute devices, so no controllers or flash drives, but also non-gpu hardware:
https://clang.llvm.org/docs/OffloadingDesign.html#openmp-offloading

supports [..] target offloading to several different architectures such as NVPTX, AMDGPU, X86_64, Arm, and PowerPC.

But then again GPUs are surely the most popular use-case (though afaik there is a hope to support other coprocessors too).
I don't care strongly for either and for some co-processors like TPUs support we probably want MLIR instead of offload anyway, which would be a follow-up project.

@bors
Copy link
Contributor

bors commented Oct 19, 2024

☔ The latest upstream changes (presumably #131934) made this pull request unmergeable. Please resolve the merge conflicts.

@ZuseZ4
Copy link
Contributor Author

ZuseZ4 commented Oct 25, 2024

ping @Kobzol

@Kobzol
Copy link
Contributor

Kobzol commented Oct 25, 2024

Oh, this had a conflict and was set to waiting-on-author, so I didn't look at it. Looks fine, please rebase it and I'll approve it.

@ZuseZ4 ZuseZ4 force-pushed the enable-llvm-offload-runtime branch from b1f21b8 to e2d3f5a Compare October 25, 2024 21:31
@ZuseZ4
Copy link
Contributor Author

ZuseZ4 commented Oct 25, 2024

@Kobzol Thanks, done.

@Kobzol
Copy link
Contributor

Kobzol commented Oct 26, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Oct 26, 2024

📌 Commit e2d3f5a has been approved by Kobzol

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 26, 2024
Zalathar added a commit to Zalathar/rust that referenced this pull request Oct 26, 2024
…, r=Kobzol

Enable the llvm offload build configuration

Tracking:

- rust-lang#131513
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 26, 2024
…r=Kobzol

Enable the llvm offload build configuration

Tracking:

- rust-lang#131513
@bors
Copy link
Contributor

bors commented Oct 26, 2024

⌛ Testing commit e2d3f5a with merge a1d37c2...

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-mingw failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
---- [run-make] tests\run-make\avr-rjmp-offset stdout ----

error: rmake recipe failed to complete
status: exit code: 1
command: "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\test\\run-make\\avr-rjmp-offset\\rmake.exe"
--- stderr -------------------------------
command failed at line 29
command failed at line 29
Command { cmd: "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\stage2\\bin\\rustc.exe" "-L" "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\test\\run-make\\avr-rjmp-offset\\rmake_out" "avr-rjmp-offsets.rs" "-Copt-level=s" "-Cpanic=abort" "--target=avr-unknown-gnu-atmega328" "-Clinker=rust-lld" "-Clink-arg=--entry=main" "-o" "compiled", stdin_buf: None, stdin: None, stdout: None, stderr: None, drop_bomb: DropBomb { command: "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\stage2\\bin\\rustc.exe", defused: true, armed_location: Location { file: "C:\\a\\rust\\rust\\tests\\run-make\\avr-rjmp-offset\\rmake.rs", line: 16, col: 5 } }, already_executed: true }
output status: `exit code: 1`
=== STDOUT ===


=== STDERR ===
error: linking with `rust-lld` failed: exit code: 0xc0000374
error: linking with `rust-lld` failed: exit code: 0xc0000374
  |
  = note: "rust-lld" "-flavor" "gnu" "C:\\a\\_temp\\msys64\\tmp\\rustc4vG81h\\symbols.o" "compiled.avr_rjmp_offsets.ab053966543a1f9f-cgu.0.rcgu.o" "--as-needed" "-Bdynamic" "-z" "noexecstack" "-L" "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\test\\run-make\\avr-rjmp-offset\\rmake_out" "-o" "compiled" "--gc-sections" "--entry=main"

error: aborting due to 1 previous error
------------------------------------------

@bors
Copy link
Contributor

bors commented Oct 26, 2024

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 26, 2024
@Kobzol
Copy link
Contributor

Kobzol commented Oct 26, 2024

That looks.. spurious, but also concerning (heap corruption error?). Let's retry.

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 26, 2024
@bors
Copy link
Contributor

bors commented Oct 26, 2024

⌛ Testing commit e2d3f5a with merge 17f8215...

@bors
Copy link
Contributor

bors commented Oct 26, 2024

☀️ Test successful - checks-actions
Approved by: Kobzol
Pushing 17f8215 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 26, 2024
@bors bors merged commit 17f8215 into rust-lang:master Oct 26, 2024
7 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Oct 26, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (17f8215): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)
0.4% [0.4%, 0.4%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.4% [0.4%, 0.4%] 1

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 782.08s -> 784.653s (0.33%)
Artifact size: 333.70 MiB -> 333.81 MiB (0.03%)

@ZuseZ4 ZuseZ4 deleted the enable-llvm-offload-runtime branch October 26, 2024 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants