Skip to content

Sync rustc_codegen_cranelift #110992

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

Merged
merged 99 commits into from
Apr 29, 2023
Merged

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Apr 29, 2023

Updated Cranelift and fixed a couple of bugs.

r? @ghost

@rustbot label +A-codegen +A-cranelift +T-compiler

cjgillot and others added 30 commits February 27, 2023 19:25
Implement checked Shl/Shr at MIR building.

This does not require any special handling by codegen backends,
as the overflow behaviour is entirely determined by the rhs (shift amount).

This allows MIR ConstProp to remove the overflow check for constant shifts.

~There is an existing different behaviour between cg_llvm and cg_clif (cc `@bjorn3).`
I took cg_llvm's one as reference: overflow if `rhs < 0 || rhs > number_of_bits_in_lhs_ty`.~

EDIT: `cg_llvm` and `cg_clif` implement the overflow check differently. This PR uses `cg_llvm`'s implementation based on a `BitAnd` instead of `cg_clif`'s one based on an unsigned comparison.
Extract and reuse logic controlling behaviour of overflow checking
assertions instead of duplicating it three times.
cranelift-frontend now uses iconst.i64 + uextend instead of the invalid
iconst.i128.
Updates `interpret`, `codegen_ssa`, and `codegen_cranelift` to consume the new cast instead of the intrinsic.

Includes `CastTransmute` for custom MIR building, to be able to test the extra UB.
Add `CastKind::Transmute` to MIR

~~Nothing actually produces it in this commit, so I don't know how to test it, but it also means it shouldn't be possible for it to break anything.~~

Includes lowering `transmute` calls to it, so it's used.

Zulip Conversation: <https://rust-lang.zulipchat.com/#narrow/stream/189540-t-compiler.2Fwg-mir-opt/topic/Good.20first.20isssue/near/321849610>
They have been causing a lot of trouble. For example current MIR
building thinks that it is fine to dynamically index into them. And
there are different paths depending on if the repr(simd) struct uses
fields or a single array as interior. There is also trouble with moving
the inner array of a repr(simd) type that is an array wrapper.

If performance becomes a concern, I will implement this in a more
principled way.
To ensure people who watch the repo for release notifications actually get a
notification.
bjorn3 and others added 12 commits April 25, 2023 12:32
This allows the user to overwrite them and prevents confusing error
messages if the last argument supplied expects a value.
They're semantically the same, so this means the backends don't need to handle the intrinsic and means fewer MIR basic blocks in pointer arithmetic code.
This version was released as part of a security fix for Wasmtime. The
fix didn't change Cranelift though, so this commit is not strictly
necessary, but also doesn't hurt.
@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc 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) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-codegen Area: Code generation A-cranelift Things relevant to the [future] cranelift backend labels Apr 29, 2023
@bjorn3
Copy link
Member Author

bjorn3 commented Apr 29, 2023

@bors r+ p=1 subtree sync

@bors
Copy link
Collaborator

bors commented Apr 29, 2023

📌 Commit d925a53 has been approved by bjorn3

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-review Status: Awaiting review from the assignee but also interested parties. labels Apr 29, 2023
@bors
Copy link
Collaborator

bors commented Apr 29, 2023

⌛ Testing commit d925a53 with merge 27d22d2...

@bors
Copy link
Collaborator

bors commented Apr 29, 2023

☀️ Test successful - checks-actions
Approved by: bjorn3
Pushing 27d22d2 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 29, 2023
@bors bors merged commit 27d22d2 into rust-lang:master Apr 29, 2023
@rustbot rustbot added this to the 1.71.0 milestone Apr 29, 2023
@bjorn3 bjorn3 deleted the sync_cg_clif-2023-04-29 branch April 29, 2023 17:15
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (27d22d2): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

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

Cycles

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation A-cranelift Things relevant to the [future] cranelift backend A-testsuite Area: The testsuite used to check the correctness of rustc 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) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.