-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
Move the libgccjit.so file in a target directory #149426
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
|
This PR changes how GCC is built. Consider updating src/bootstrap/download-ci-gcc-stamp. Some changes occurred in compiler/rustc_codegen_gcc |
|
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1fea3d0 to
3dc76dc
Compare
|
This PR modifies |
This comment has been minimized.
This comment has been minimized.
|
@bjorn3: Is there any way to bless the UI tests for Aarch64 from an x86 computer? |
|
You can try if |
| //@ add-minicore | ||
| //@ compile-flags: --target thumbv8m.main-none-eabi --crate-type lib | ||
| //@ needs-llvm-components: arm | ||
| //@ ignore-backends: gcc |
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.
All these new test failures are due to the fact that --target is used and we now put libgccjit.so in a directory named after the target, so rustc_codegen_gcc doesn't find libgccjit.so.
Is this the correct way to ignore these tests?
Or are tests annotated with needs-llvm-compoments not expected to pass with a codegen that is not LLVM?
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 think //@ ignore-backends: gcc makes sense. These tests would pass just fine with Cranelift if it is one of the targets supported by Cranelift. cg_clif always enables all backends of Cranelift.
|
☔ The latest upstream changes (presumably #149560) made this pull request unmergeable. Please resolve the merge conflicts. |
8b77a86 to
1935f8c
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1935f8c to
c219930
Compare
|
☔ The latest upstream changes (presumably #149642) made this pull request unmergeable. Please resolve the merge conflicts. |
Since GCC is not multi-target, we need multiple libgccjit.so. Our solution to have a directory per target so that we can have multiple libgccjit.so.
c219930 to
42059a3
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
The bootstrap changes look fine. I also understand why some tests might need reblessing. But I don't understand how this can break tests that worked previously with GCC? cg_gcc should work the same as before, right? I tried it locally, but it panicked on this: |
This one is likely because you forgot to disable debug assertions. The new tests failure are only caused because they specify a |
I see. But before, cg_gcc definitely didn't have the corresponding libgccjit file for these targets. So it should have failed too, right? Just with a different error message. Why didn't the tests fail before? Were those silent failures? |
It would load the |
|
I see. Yeah, if it never got to the code generation stage, then it makes sense that it wouldn't fail before.
It seems to be called unconditionally right at the beginning of the compilation. |
|
This looks reasonable. Let's see if it goes through full CI: @bors r+ rollup=never |
|
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing 0b96731 (parent) -> a371038 (this PR) Test differencesShow 27 test diffsStage 2
Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard a371038013f4f3e3f1d4fdcacfaa02c3252a518b --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (a371038): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (primary -2.7%, secondary 3.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 2.7%, secondary 6.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 471.709s -> 470.343s (-0.29%) |
Since GCC is not multi-target, we need multiple libgccjit.so. Our solution to have a directory per target so that we can have multiple libgccjit.so.
r? @Kobzol