Skip to content

the unadjusted ABI needs to pass aggregates by-value#118127

Merged
bors merged 2 commits intorust-lang:masterfrom
RalfJung:unadjusted-abi
Nov 25, 2023
Merged

the unadjusted ABI needs to pass aggregates by-value#118127
bors merged 2 commits intorust-lang:masterfrom
RalfJung:unadjusted-abi

Conversation

@RalfJung
Copy link
Member

@RalfJung RalfJung commented Nov 21, 2023 •
edited
Loading

Fixes #118124, a regression introduced in #117500

@rustbot
Copy link
Collaborator

rustbot commented Nov 21, 2023

r? @davidtwco

(rustbot has picked a reviewer for you, use r? to override)

@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. labels Nov 21, 2023
@RalfJung RalfJung mentioned this pull request Nov 21, 2023
Closed
@Noratrieb
Copy link
Member

Noratrieb commented Nov 21, 2023

Could you just add a test that uses the NEON intrinsic and ensures it doesn't fail to compile?

@RalfJung
Copy link
Member Author

RalfJung commented Nov 21, 2023

I can probably do a no_core test that works on all hosts...

@bjorn3
Copy link
Member

bjorn3 commented Nov 21, 2023

Is the unadjusted ABI marked as internal feature? If not I think we should mark it as such.

@RalfJung
Copy link
Member Author

RalfJung commented Nov 21, 2023

Not currently:

rust/compiler/rustc_feature/src/unstable.rs

Lines 162 to 163 in efe54e2

/// Allows using the `unadjusted` ABI; perma-unstable.
(unstable, abi_unadjusted, "1.16.0", None, None),

But I agree.

@RalfJung RalfJung mentioned this pull request Nov 22, 2023
Closed
@RalfJung RalfJung mentioned this pull request Nov 22, 2023
Merged
@RalfJung
Copy link
Member Author

RalfJung commented Nov 22, 2023

Could you just add a test that uses the NEON intrinsic and ensures it doesn't fail to compile?

I added a test.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 23, 2023
@matthiaskrgr
Rollup merge of rust-lang#118167 - RalfJung:unadjusted-abi-is-interna…
9e944c8 
…l, r=petrochenkov

make the 'abi_unadjusted' feature internal

As [suggested](rust-lang#118127 (comment)) by `@bjorn3.`
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 23, 2023
@rust-timer
Unrolled build for rust-lang#118167
46522a1 
Rollup merge of rust-lang#118167 - RalfJung:unadjusted-abi-is-internal, r=petrochenkov

make the 'abi_unadjusted' feature internal

As [suggested](rust-lang#118127 (comment)) by `@bjorn3.`
@RalfJung
Copy link
Member Author

RalfJung commented Nov 25, 2023

@davidtwco or anyone else from @rust-lang/compiler , this fixes a critical codegen regression, would be good to land it ASAP. :)

@ytmimi ytmimi mentioned this pull request Nov 25, 2023
Closed
@compiler-errors
Copy link
Contributor

compiler-errors commented Nov 25, 2023

@bors r+ rollup=never p=1

easier to bisect that way

@bors
Copy link
Collaborator

bors commented Nov 25, 2023

📌 Commit ebfb95a has been approved by compiler-errors

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 Nov 25, 2023
@bors
Copy link
Collaborator

bors commented Nov 25, 2023

⌛ Testing commit ebfb95a with merge 16087ee...

@bors
Copy link
Collaborator

bors commented Nov 25, 2023

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 16087ee to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 25, 2023
@bors bors merged commit 16087ee into rust-lang:master Nov 25, 2023
@rustbot rustbot added this to the 1.76.0 milestone Nov 25, 2023
@rust-timer
Copy link
Collaborator

rust-timer commented Nov 25, 2023

Finished benchmarking commit (16087ee): 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)		 4.7% [4.7%, 4.7%] 1
Regressions ❌
(secondary)		 2.6% [2.6%, 2.6%] 1
Improvements ✅
(primary)		 -3.9% [-3.9%, -3.9%] 1
Improvements ✅
(secondary)		 -3.7% [-4.8%, -2.2%] 4
All ❌✅ (primary) 0.4% [-3.9%, 4.7%] 2

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: 675.133s -> 674.958s (-0.03%)
Artifact size: 313.36 MiB -> 313.33 MiB (-0.01%)

@RalfJung RalfJung deleted the unadjusted-abi branch November 27, 2023 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@davidtwco davidtwco

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-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.76.0

Development

Successfully merging this pull request may close these issues.

Intrinsic has incorrect return type. LLVM ERROR: Broken function found, compilation aborted. aarch64 / ARM / nightly

8 participants

@RalfJung @rustbot @Noratrieb @bjorn3 @compiler-errors @bors @rust-timer @davidtwco