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

ABI checks: add support for tier2 arches #132842

Merged
merged 1 commit into from
Nov 14, 2024

Conversation

veluca93
Copy link
Contributor

See #131800 for the data collection behind this change.

r? RalfJung

@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 10, 2024
@RalfJung
Copy link
Member

Sorry, I'm going to hand this one off.

r? compiler

@rustbot rustbot assigned cjgillot and unassigned RalfJung Nov 10, 2024
@RalfJung
Copy link
Member

In terms of reviewers, I guess rolling the dice is unlikely to get an ABI person. ;)

r? @workingjubilee

@veluca93 veluca93 force-pushed the abi-checks-tier2 branch 2 times, most recently from 1cc2c1a to ff485c9 Compare November 10, 2024 16:39
compiler/rustc_target/src/target_features.rs Show resolved Hide resolved
compiler/rustc_target/src/target_features.rs Outdated Show resolved Hide resolved
compiler/rustc_target/src/target_features.rs Outdated Show resolved Hide resolved
compiler/rustc_target/src/target_features.rs Outdated Show resolved Hide resolved
@bors
Copy link
Contributor

bors commented Nov 12, 2024

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

@bors
Copy link
Contributor

bors commented Nov 12, 2024

✌️ @veluca93, you can now approve this pull request!

If @workingjubilee told you to "r=me" after making some further change, please make that change, then do @bors r=@workingjubilee

See rust-lang#131800 for the data collection behind this change.

Also adds a test that exercise the "empty list of features" path.
@veluca93
Copy link
Contributor Author

Added all the suggested comments (in one form or the other :-))

@bors r=@workingjubilee

@bors
Copy link
Contributor

bors commented Nov 12, 2024

📌 Commit 295cffc has been approved by workingjubilee

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 12, 2024
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Nov 13, 2024
…ngjubilee

ABI checks: add support for tier2 arches

See rust-lang#131800 for the data collection behind this change.

r? RalfJung
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 13, 2024
…llaumeGomez

Rollup of 6 pull requests

Successful merges:

 - rust-lang#132709 (optimize char::to_digit and assert radix is at least 2)
 - rust-lang#132842 (ABI checks: add support for tier2 arches)
 - rust-lang#132965 (allow CFGuard on windows-gnullvm)
 - rust-lang#132967 (fix REGISTRY_USERNAME to reuse cache between auto and pr jobs)
 - rust-lang#132971 (Handle infer vars in anon consts on stable)
 - rust-lang#132979 (use `--exact` on `--skip` to avoid unintended substring matches)

r? `@ghost`
`@rustbot` modify labels: rollup
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Nov 13, 2024
…ngjubilee

ABI checks: add support for tier2 arches

See rust-lang#131800 for the data collection behind this change.

r? RalfJung
const RISCV_FEATURES_FOR_CORRECT_VECTOR_ABI: &'static [(u64, &'static str)] =
&[/*(64, "zvl64b"), */ (128, "v")];
// Always warn on SPARC, as the necessary target features cannot be enabled in Rust at the moment.
const SPARC_FEATURES_FOR_CORRECT_VECTOR_ABI: &'static [(u64, &'static str)] = &[/*(128, "vis")*/];
Copy link
Member

Choose a reason for hiding this comment

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

What is the source for the 128 here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

@workingjubilee workingjubilee Nov 13, 2024

Choose a reason for hiding this comment

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

checks I could've sworn it was 128-bit, but apparently it's only 64-bit.

Copy link
Member

Choose a reason for hiding this comment

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

This is already rolled up so let's let it land, and then @veluca93 can you prepare a follow-up to fix the comment?

Copy link
Member

Choose a reason for hiding this comment

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

I know one of the SPARC CPUs has 128-bit registers, but I guess I was misremembering what extension defines that.

Copy link
Member

Choose a reason for hiding this comment

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

I think we can get it in the tier 3 pass, but yeah, it's just a comment.

I probably should've just skipped noting down a size at all.

Copy link
Member

Choose a reason for hiding this comment

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

I think we can get it in the tier 3 pass,

That also works.

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 13, 2024
…llaumeGomez

Rollup of 5 pull requests

Successful merges:

 - rust-lang#132709 (optimize char::to_digit and assert radix is at least 2)
 - rust-lang#132842 (ABI checks: add support for tier2 arches)
 - rust-lang#132965 (allow CFGuard on windows-gnullvm)
 - rust-lang#132967 (fix REGISTRY_USERNAME to reuse cache between auto and pr jobs)
 - rust-lang#132971 (Handle infer vars in anon consts on stable)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 13, 2024
…llaumeGomez

Rollup of 5 pull requests

Successful merges:

 - rust-lang#132709 (optimize char::to_digit and assert radix is at least 2)
 - rust-lang#132842 (ABI checks: add support for tier2 arches)
 - rust-lang#132965 (allow CFGuard on windows-gnullvm)
 - rust-lang#132967 (fix REGISTRY_USERNAME to reuse cache between auto and pr jobs)
 - rust-lang#132971 (Handle infer vars in anon consts on stable)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 13, 2024
…llaumeGomez

Rollup of 5 pull requests

Successful merges:

 - rust-lang#132709 (optimize char::to_digit and assert radix is at least 2)
 - rust-lang#132842 (ABI checks: add support for tier2 arches)
 - rust-lang#132965 (allow CFGuard on windows-gnullvm)
 - rust-lang#132967 (fix REGISTRY_USERNAME to reuse cache between auto and pr jobs)
 - rust-lang#132971 (Handle infer vars in anon consts on stable)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 14, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#126046 (Implement `mixed_integer_ops_unsigned_sub`)
 - rust-lang#132302 (rustdoc: Treat declarative macros more like other item kinds)
 - rust-lang#132842 (ABI checks: add support for tier2 arches)
 - rust-lang#132995 (compiletest: Add ``exact-llvm-major-version`` directive)
 - rust-lang#132996 (Trim extra space when suggesting removing bad `let`)
 - rust-lang#132998 (Unvacation myself)
 - rust-lang#133000 ([rustdoc] Fix duplicated footnote IDs)
 - rust-lang#133001 (actually test next solver)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a5372ed into rust-lang:master Nov 14, 2024
6 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Nov 14, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 14, 2024
Rollup merge of rust-lang#132842 - veluca93:abi-checks-tier2, r=workingjubilee

ABI checks: add support for tier2 arches

See rust-lang#131800 for the data collection behind this change.

r? RalfJung
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 16, 2024
…jubilee

ABI checks: add support for some tier3 arches, warn on others.

Followup to
- rust-lang#132842
- rust-lang#132173
- rust-lang#131800

r? `@workingjubilee`
jhpratt added a commit to jhpratt/rust that referenced this pull request Nov 17, 2024
…ngjubilee

ABI checks: add support for some tier3 arches, warn on others.

Followup to
- rust-lang#132842
- rust-lang#132173
- rust-lang#131800

r? `@workingjubilee`
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Nov 17, 2024
…ngjubilee

ABI checks: add support for some tier3 arches, warn on others.

Followup to
- rust-lang#132842
- rust-lang#132173
- rust-lang#131800

r? ``@workingjubilee``
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 17, 2024
Rollup merge of rust-lang#133029 - veluca93:abi-checks-tier3, r=workingjubilee

ABI checks: add support for some tier3 arches, warn on others.

Followup to
- rust-lang#132842
- rust-lang#132173
- rust-lang#131800

r? ``@workingjubilee``
@@ -1,12 +1,19 @@
monomorphize_abi_error_disabled_vector_type_call =
ABI error: this function call uses a vector type that requires the `{$required_feature}` target feature, which is not enabled in the caller
this function call uses a SIMD vector type that (with the chosen ABI) requires the `{$required_feature}` target feature, which is not enabled in the caller
.label = function called here
.help = consider enabling it globally (`-C target-feature=+{$required_feature}`) or locally (`#[target_feature(enable="{$required_feature}")]`)
Copy link

@riking riking Nov 26, 2024

Choose a reason for hiding this comment

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

I think we should add a second line of help: consider passing a reference to the value instead

Copy link
Member

Choose a reason for hiding this comment

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

Sure if you control both sides of the FFI you can do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

8 participants