Skip to content

Subtree sync for rustc_codegen_cranelift #127694

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 21 commits into from
Jul 14, 2024

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Jul 13, 2024

Couple of bug fixes this time.

Fixes #125545

r? @ghost

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

bjorn3 and others added 21 commits June 30, 2024 11:28
Miri function identity hack: account for possible inlining

Having a non-lifetime generic is not the only reason a function can be duplicated. Another possibility is that the function may be eligible for cross-crate inlining. So also take into account the inlining attribute in this Miri hack for function pointer identity.

That said, `cross_crate_inlinable` will still sometimes return true even for `inline(never)` functions:
- when they are `DefKind::Ctor(..) | DefKind::Closure` -- I assume those cannot be `InlineAttr::Never` anyway?
- when `cross_crate_inline_threshold == InliningThreshold::Always`

so maybe this is still not quite the right criterion to use for function pointer identity.
…, r=oli-obk,RalfJung

Support tail calls in mir via `TerminatorKind::TailCall`

This is one of the interesting bits in tail call implementation — MIR support.

This adds a new `TerminatorKind` which represents a tail call:
```rust
    TailCall {
        func: Operand<'tcx>,
        args: Vec<Operand<'tcx>>,
        fn_span: Span,
    },
```

*Structurally* this is very similar to a normal `Call` but is missing a few fields:
- `destination` — tail calls don't write to destination, instead they pass caller's destination to the callee (such that eventual `return` will write to the caller of the function that used tail call)
- `target` — similarly to `destination` tail calls pass the caller's return address to the callee, so there is nothing to do
- `unwind` — I _think_ this is applicable too, although it's a bit confusing
- `call_source` — `become` forbids operators and is not created as a lowering of something else; tail calls always come from HIR (at least for now)

It might be helpful to read the interpreter implementation to understand what `TailCall` means exactly, although I've tried documenting it too.

-----

There are a few `FIXME`-questions still left, ideally we'd be able to answer them during review ':)

-----

r? `@oli-obk`
cc `@scottmcm` `@DrMeepster` `@JakobDegen`
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 13, 2024
@rustbot
Copy link
Collaborator

rustbot commented Jul 13, 2024

There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

You can start a rebase with the following commands:

$ # rebase
$ git pull --rebase https://github.com/rust-lang/rust.git master
$ git push --force-with-lease

The following commits are merge commits:

@rustbot rustbot added A-codegen Area: Code generation A-cranelift Things relevant to the [future] cranelift backend labels Jul 13, 2024
@bjorn3
Copy link
Member Author

bjorn3 commented Jul 13, 2024

@bors r+ p=1 subtree sync

@bors
Copy link
Collaborator

bors commented Jul 13, 2024

📌 Commit 6ff363f 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. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 13, 2024
@bors
Copy link
Collaborator

bors commented Jul 13, 2024

⌛ Testing commit 6ff363f with merge 00167ab...

@bors
Copy link
Collaborator

bors commented Jul 14, 2024

☀️ Test successful - checks-actions
Approved by: bjorn3
Pushing 00167ab to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 14, 2024
@bors bors merged commit 00167ab into rust-lang:master Jul 14, 2024
7 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jul 14, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (00167ab): 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 (primary 6.1%)

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)
6.1% [6.1%, 6.1%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 6.1% [6.1%, 6.1%] 1

Cycles

Results (secondary 2.9%)

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)
2.9% [2.9%, 2.9%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 705.014s -> 706.105s (0.15%)
Artifact size: 328.67 MiB -> 328.68 MiB (0.00%)

@bjorn3 bjorn3 deleted the sync_cg_clif-2024-07-13 branch July 14, 2024 12:16
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 has-merge-commits PR has merge commits, merge with caution. 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-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.

Nightly rustc panic when compiling a simple no_std program
7 participants