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

After #106856, CARGO_CFG_TARGET_HAS_ATOMIC{,_LOAD_STORE} become empty strings #108201

Closed
taiki-e opened this issue Feb 18, 2023 · 4 comments
Closed
Labels
C-bug Category: This is a bug. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-cargo Relevant to the cargo team, which will review and decide on the PR/issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@taiki-e
Copy link
Member

taiki-e commented Feb 18, 2023

Code

I ran cargo build -vv with this code:

# Cargo.toml
[package]
name = "repro"
version = "0.1.0"
edition = "2021"
// build.rs
fn main() {
    dbg!(std::env::var("CARGO_CFG_TARGET_HAS_ATOMIC").ok());
    dbg!(std::env::var("CARGO_CFG_TARGET_HAS_ATOMIC_LOAD_STORE").ok());
    dbg!(std::env::var("CARGO_CFG_TARGET_HAS_ATOMIC_EQUAL_ALIGNMENT").ok());
}

I expected to see this happen:

[repro 0.1.0] [build.rs:2] std::env::var("CARGO_CFG_TARGET_HAS_ATOMIC").ok() = Some(
[repro 0.1.0]     "128,16,32,64,8,ptr",
[repro 0.1.0] )
[repro 0.1.0] [build.rs:3] std::env::var("CARGO_CFG_TARGET_HAS_ATOMIC_LOAD_STORE").ok() = Some(
[repro 0.1.0]     "128,16,32,64,8,ptr",
[repro 0.1.0] )
[repro 0.1.0] [build.rs:4] std::env::var("CARGO_CFG_TARGET_HAS_ATOMIC_EQUAL_ALIGNMENT").ok() = Some(
[repro 0.1.0]     "128,16,32,64,8,ptr",
[repro 0.1.0] )

Instead, this happened:

[repro 0.1.0] [build.rs:2] std::env::var("CARGO_CFG_TARGET_HAS_ATOMIC").ok() = Some(
[repro 0.1.0]     "",
[repro 0.1.0] )
[repro 0.1.0] [build.rs:3] std::env::var("CARGO_CFG_TARGET_HAS_ATOMIC_LOAD_STORE").ok() = Some(
[repro 0.1.0]     "",
[repro 0.1.0] )
[repro 0.1.0] [build.rs:4] std::env::var("CARGO_CFG_TARGET_HAS_ATOMIC_EQUAL_ALIGNMENT").ok() = Some(
[repro 0.1.0]     "128,16,32,64,8,ptr",
[repro 0.1.0] )

It seems that cfg(target_has_atomic{,_load_store}) without value that is added in #106856 is overriding cfg(target_has_atomic{,_load_store}) with value.

This environment variable still seems to be rarely used in the ecosystem, but profiler_builtins uses it.

if env::var_os("CARGO_CFG_TARGET_HAS_ATOMIC")

Mentioning @vadorovsky who authored #106856

Version it worked on

It most recently worked on: nightly-2023-01-27

$ rustc +nightly-2023-01-27 --print cfg | grep target_has_atomic
target_has_atomic="128"
target_has_atomic="16"
target_has_atomic="32"
target_has_atomic="64"
target_has_atomic="8"
target_has_atomic="ptr"
target_has_atomic_equal_alignment="128"
target_has_atomic_equal_alignment="16"
target_has_atomic_equal_alignment="32"
target_has_atomic_equal_alignment="64"
target_has_atomic_equal_alignment="8"
target_has_atomic_equal_alignment="ptr"
target_has_atomic_load_store="128"
target_has_atomic_load_store="16"
target_has_atomic_load_store="32"
target_has_atomic_load_store="64"
target_has_atomic_load_store="8"
target_has_atomic_load_store="ptr"

Version with regression

nightly-2023-01-28
rustc --version --verbose:

rustc 1.69.0-nightly (ef982929c 2023-01-27)
binary: rustc
commit-hash: ef982929c0b653436a6ea6892a2a839fba7c8b57
commit-date: 2023-01-27
host: aarch64-apple-darwin
release: 1.69.0-nightly
LLVM version: 15.0.7
$ rustc +nightly-2023-01-28 --print cfg | grep target_has_atomic
target_has_atomic
target_has_atomic="128"
target_has_atomic="16"
target_has_atomic="32"
target_has_atomic="64"
target_has_atomic="8"
target_has_atomic="ptr"
target_has_atomic_equal_alignment="128"
target_has_atomic_equal_alignment="16"
target_has_atomic_equal_alignment="32"
target_has_atomic_equal_alignment="64"
target_has_atomic_equal_alignment="8"
target_has_atomic_equal_alignment="ptr"
target_has_atomic_load_store
target_has_atomic_load_store="128"
target_has_atomic_load_store="16"
target_has_atomic_load_store="32"
target_has_atomic_load_store="64"
target_has_atomic_load_store="8"
target_has_atomic_load_store="ptr"

@rustbot modify labels: +regression-from-stable-to-nightly -regression-untriaged

@taiki-e taiki-e added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Feb 18, 2023
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. and removed regression-untriaged Untriaged performance or correctness regression. labels Feb 18, 2023
@vadorovsky
Copy link
Contributor

Thanks for the report! I will try to fix that soon.

@apiraino apiraino added T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Feb 21, 2023
@m-ou-se m-ou-se added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-cargo Relevant to the cargo team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 22, 2023
@vadorovsky
Copy link
Contributor

@taiki-e Sorry for following up so late. I finally managed to debug it properly and I think it's an issue which should be fixed in Cargo rather than rustc. I created the issue and I will make a PR today:

rust-lang/cargo#11789

Please let me know if you agree with that approach. I think we should support a mix of configs with and without value - that works nicely for differentiating between "check for atomix for type X" and "check for atomics in general" in rustc.

@taiki-e
Copy link
Member Author

taiki-e commented Mar 2, 2023

Thanks. That approach looks reasonable to me.

@taiki-e
Copy link
Member Author

taiki-e commented May 9, 2023

Fixed in rust-lang/cargo#11790. Thanks @vadorovsky.

@taiki-e taiki-e closed this as completed May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-cargo Relevant to the cargo team, which will review and decide on the PR/issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants