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

s390x: Fix a regression related to backchain feature #129940

Merged
merged 2 commits into from
Sep 7, 2024

Conversation

liushuyu
Copy link
Contributor

@liushuyu liushuyu commented Sep 3, 2024

In #127506, we introduced a new IBM Z-specific target feature, backchain.

This particular target-feature was available as a function-level attribute in LLVM 17 and below, so some hacks were used to avoid blowing up LLVM when querying the supported LLVM features.

This led to an unfortunate regression where cfg!(target-feature = "backchain") will always return true.

This pull request aims to fix this issue, and a test has been introduced to ensure it will never happen again.

Fixes #129927.

r? @RalfJung

... can not be correctly gated using #[cfg] macro
@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 Sep 3, 2024
@liushuyu
Copy link
Contributor Author

liushuyu commented Sep 3, 2024

@rustbot label O-SystemZ A-LLVM A-target-feature C-bug

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. C-bug Category: This is a bug. O-SystemZ Target: SystemZ processors (s390x) labels Sep 3, 2024
@RalfJung
Copy link
Member

RalfJung commented Sep 4, 2024 via email

@bors
Copy link
Contributor

bors commented Sep 4, 2024

📌 Commit 6e4c5c1 has been approved by RalfJung

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 Sep 4, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 5, 2024
…RalfJung

s390x: Fix a regression related to backchain feature

In rust-lang#127506, we introduced a new IBM Z-specific target feature, `backchain`.

This particular `target-feature` was available as a function-level attribute in LLVM 17 and below, so some hacks were used to avoid blowing up LLVM when querying the supported LLVM features.

This led to an unfortunate regression where `cfg!(target-feature = "backchain")` will always return true.

This pull request aims to fix this issue, and a test has been introduced to ensure it will never happen again.

Fixes rust-lang#129927.

r? `@RalfJung`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 5, 2024
…RalfJung

s390x: Fix a regression related to backchain feature

In rust-lang#127506, we introduced a new IBM Z-specific target feature, `backchain`.

This particular `target-feature` was available as a function-level attribute in LLVM 17 and below, so some hacks were used to avoid blowing up LLVM when querying the supported LLVM features.

This led to an unfortunate regression where `cfg!(target-feature = "backchain")` will always return true.

This pull request aims to fix this issue, and a test has been introduced to ensure it will never happen again.

Fixes rust-lang#129927.

r? ``@RalfJung``
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 5, 2024
…iaskrgr

Rollup of 12 pull requests

Successful merges:

 - rust-lang#128919 (Add an internal lint that warns when accessing untracked data)
 - rust-lang#129471 ([rustdoc] Sort impl associated items by kinds and then by appearance)
 - rust-lang#129653 (clarify that addr_of creates read-only pointers)
 - rust-lang#129775 (bootstrap: Try to track down why `initial_libdir` sometimes fails)
 - rust-lang#129939 (explain why Rvalue::Len still exists)
 - rust-lang#129940 (s390x: Fix a regression related to backchain feature)
 - rust-lang#129942 (copy rustc rustlib artifacts from ci-rustc)
 - rust-lang#129943 (use the bootstrapped compiler for `test-float-parse` test)
 - rust-lang#129944 (Add compat note for trait solver change)
 - rust-lang#129947 (Add digit separators in `Duration` examples)
 - rust-lang#129955 (Temporarily remove fmease from the review rotation)
 - rust-lang#129957 (forward linker option to lint-docs)

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

Rollup of 12 pull requests

Successful merges:

 - rust-lang#128919 (Add an internal lint that warns when accessing untracked data)
 - rust-lang#129471 ([rustdoc] Sort impl associated items by kinds and then by appearance)
 - rust-lang#129653 (clarify that addr_of creates read-only pointers)
 - rust-lang#129775 (bootstrap: Try to track down why `initial_libdir` sometimes fails)
 - rust-lang#129939 (explain why Rvalue::Len still exists)
 - rust-lang#129940 (s390x: Fix a regression related to backchain feature)
 - rust-lang#129942 (copy rustc rustlib artifacts from ci-rustc)
 - rust-lang#129943 (use the bootstrapped compiler for `test-float-parse` test)
 - rust-lang#129944 (Add compat note for trait solver change)
 - rust-lang#129947 (Add digit separators in `Duration` examples)
 - rust-lang#129955 (Temporarily remove fmease from the review rotation)
 - rust-lang#129957 (forward linker option to lint-docs)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 5, 2024
…RalfJung

s390x: Fix a regression related to backchain feature

In rust-lang#127506, we introduced a new IBM Z-specific target feature, `backchain`.

This particular `target-feature` was available as a function-level attribute in LLVM 17 and below, so some hacks were used to avoid blowing up LLVM when querying the supported LLVM features.

This led to an unfortunate regression where `cfg!(target-feature = "backchain")` will always return true.

This pull request aims to fix this issue, and a test has been introduced to ensure it will never happen again.

Fixes rust-lang#129927.

r? `@RalfJung`
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 5, 2024
…iaskrgr

Rollup of 12 pull requests

Successful merges:

 - rust-lang#128919 (Add an internal lint that warns when accessing untracked data)
 - rust-lang#129472 (fix ICE when `asm_const` and `const_refs_to_static` are combined)
 - rust-lang#129653 (clarify that addr_of creates read-only pointers)
 - rust-lang#129775 (bootstrap: Try to track down why `initial_libdir` sometimes fails)
 - rust-lang#129939 (explain why Rvalue::Len still exists)
 - rust-lang#129940 (s390x: Fix a regression related to backchain feature)
 - rust-lang#129942 (copy rustc rustlib artifacts from ci-rustc)
 - rust-lang#129943 (use the bootstrapped compiler for `test-float-parse` test)
 - rust-lang#129944 (Add compat note for trait solver change)
 - rust-lang#129947 (Add digit separators in `Duration` examples)
 - rust-lang#129955 (Temporarily remove fmease from the review rotation)
 - rust-lang#129957 (forward linker option to lint-docs)

Failed merges:

 - rust-lang#129471 ([rustdoc] Sort impl associated items by kinds and then by appearance)

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

Rollup of 12 pull requests

Successful merges:

 - rust-lang#128919 (Add an internal lint that warns when accessing untracked data)
 - rust-lang#129472 (fix ICE when `asm_const` and `const_refs_to_static` are combined)
 - rust-lang#129653 (clarify that addr_of creates read-only pointers)
 - rust-lang#129775 (bootstrap: Try to track down why `initial_libdir` sometimes fails)
 - rust-lang#129939 (explain why Rvalue::Len still exists)
 - rust-lang#129940 (s390x: Fix a regression related to backchain feature)
 - rust-lang#129942 (copy rustc rustlib artifacts from ci-rustc)
 - rust-lang#129943 (use the bootstrapped compiler for `test-float-parse` test)
 - rust-lang#129944 (Add compat note for trait solver change)
 - rust-lang#129947 (Add digit separators in `Duration` examples)
 - rust-lang#129955 (Temporarily remove fmease from the review rotation)
 - rust-lang#129957 (forward linker option to lint-docs)

Failed merges:

 - rust-lang#129471 ([rustdoc] Sort impl associated items by kinds and then by appearance)

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

Rollup of 12 pull requests

Successful merges:

 - rust-lang#128919 (Add an internal lint that warns when accessing untracked data)
 - rust-lang#129472 (fix ICE when `asm_const` and `const_refs_to_static` are combined)
 - rust-lang#129653 (clarify that addr_of creates read-only pointers)
 - rust-lang#129775 (bootstrap: Try to track down why `initial_libdir` sometimes fails)
 - rust-lang#129939 (explain why Rvalue::Len still exists)
 - rust-lang#129940 (s390x: Fix a regression related to backchain feature)
 - rust-lang#129942 (copy rustc rustlib artifacts from ci-rustc)
 - rust-lang#129943 (use the bootstrapped compiler for `test-float-parse` test)
 - rust-lang#129944 (Add compat note for trait solver change)
 - rust-lang#129947 (Add digit separators in `Duration` examples)
 - rust-lang#129955 (Temporarily remove fmease from the review rotation)
 - rust-lang#129957 (forward linker option to lint-docs)

Failed merges:

 - rust-lang#129471 ([rustdoc] Sort impl associated items by kinds and then by appearance)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr
Copy link
Member

@bors r-
might cause a crash here? #129984 (comment)

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 5, 2024
@liushuyu
Copy link
Contributor Author

liushuyu commented Sep 5, 2024

@bors r- might cause a crash here? #129984 (comment)

I can't reproduce it on my local device (Linux). Unfortunately, I don't own an Apple device to test the issue

@matthiaskrgr
Copy link
Member

mmh, lets see if this can land on its own then

@bors r=RalfJung rollup=iffy

@bors
Copy link
Contributor

bors commented Sep 5, 2024

📌 Commit 6e4c5c1 has been approved by RalfJung

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 5, 2024
@RalfJung
Copy link
Member

RalfJung commented Sep 5, 2024

Sounds unlikely that this PR would lead to an LLVM crash... I guess we'll see.

bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 7, 2024
…mpiler-errors

Rollup of 9 pull requests

Successful merges:

 - rust-lang#128871 (bypass linker configuration and cross target check for specific commands)
 - rust-lang#129468 ([testsuite][cleanup] Remove all usages of `dont_merge` hack to avoid function merging)
 - rust-lang#129614 (Adjust doc comment of Condvar::wait_while)
 - rust-lang#129840 (Implement suggestions for `elided_named_lifetimes`)
 - rust-lang#129891 (Do not request sanitizers for naked functions)
 - rust-lang#129899 (Add Suggestions for Misspelled Keywords)
 - rust-lang#129940 (s390x: Fix a regression related to backchain feature)
 - rust-lang#129987 (Don't store region in `CapturedPlace`)
 - rust-lang#130054 (Add missing quotation marks)

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

Rollup of 9 pull requests

Successful merges:

 - rust-lang#128871 (bypass linker configuration and cross target check for specific commands)
 - rust-lang#129468 ([testsuite][cleanup] Remove all usages of `dont_merge` hack to avoid function merging)
 - rust-lang#129614 (Adjust doc comment of Condvar::wait_while)
 - rust-lang#129840 (Implement suggestions for `elided_named_lifetimes`)
 - rust-lang#129891 (Do not request sanitizers for naked functions)
 - rust-lang#129899 (Add Suggestions for Misspelled Keywords)
 - rust-lang#129940 (s390x: Fix a regression related to backchain feature)
 - rust-lang#129987 (Don't store region in `CapturedPlace`)
 - rust-lang#130054 (Add missing quotation marks)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit bc2244f into rust-lang:master Sep 7, 2024
6 checks passed
@rustbot rustbot added this to the 1.83.0 milestone Sep 7, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 7, 2024
Rollup merge of rust-lang#129940 - liushuyu:s390x-target-features, r=RalfJung

s390x: Fix a regression related to backchain feature

In rust-lang#127506, we introduced a new IBM Z-specific target feature, `backchain`.

This particular `target-feature` was available as a function-level attribute in LLVM 17 and below, so some hacks were used to avoid blowing up LLVM when querying the supported LLVM features.

This led to an unfortunate regression where `cfg!(target-feature = "backchain")` will always return true.

This pull request aims to fix this issue, and a test has been introduced to ensure it will never happen again.

Fixes rust-lang#129927.

r? `@RalfJung`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. C-bug Category: This is a bug. O-SystemZ Target: SystemZ processors (s390x) 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.

s390x-unknown-linux-gnu: cfg!(target_feature = "backchain") is always true
5 participants