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

Tracking Issue: fetch_nand #13226

Closed
alexcrichton opened this Issue Mar 31, 2014 · 10 comments

Comments

Projects
None yet
6 participants
@alexcrichton
Copy link
Member

alexcrichton commented Mar 31, 2014

Tracking issue for fetch_nand on integer atomics (it's stable for bool).

On android, it appears that these functions give the wrong answer. See #12964 (comment) for some analysis, and the relevant logs are:

    #[test]
    fn uint_nand() {
        let x = AtomicUint::new(0xf731);
        assert_eq!(x.fetch_nand(0x137f, SeqCst), 0xf731);
        assert_eq!(x.load(SeqCst), !(0xf731 & 0x137f));
    }

    #[test]
    fn int_nand() {
        let x = AtomicInt::new(0xf731);
        assert_eq!(x.fetch_nand(0x137f, SeqCst), 0xf731);
        assert_eq!(x.load(SeqCst), !(0xf731 & 0x137f));
    }
---- sync::atomics::test::int_nand stdout ----
    task 'sync::atomics::test::int_nand' failed at 'assertion failed: `(left == right) && (right == left)` (left: `78`, right: `-4914`)', /home/rustbuild/src/rust-buildbot/slave/auto-linux-64-x-android-t/build/src/libstd/sync/atomics.rs:1005

---- sync::atomics::test::uint_nand stdout ----
    task 'sync::atomics::test::uint_nand' failed at 'assertion failed: `(left == right) && (right == left)` (left: `78`, right: `4294962382`)', /home/rustbuild/src/rust-buildbot/slave/auto-linux-64-x-android-t/build/src/libstd/sync/atomics.rs:977  
@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Apr 28, 2015

Triage: i don't have android to test.

@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Jan 3, 2017

Triage: same as in 2015.

@steveklabnik steveklabnik added T-libs and removed A-libs labels Mar 24, 2017

@Mark-Simulacrum

This comment has been minimized.

Copy link
Member

Mark-Simulacrum commented Jun 15, 2017

I unfortunately do not have an android either, and after spending multiple hours trying to get an emulator setup I have decided to not try any more... the code below I believe is the test for this problem today. If someone has a working android version, and can test it, that would be great.

#![feature(core_intrinsics)]

use std::sync::atomic::{AtomicUsize, AtomicIsize};
use std::sync::atomic::Ordering::*;
use std::intrinsics::atomic_nand;

#[test]
fn uint_nand() {
    let mut x = AtomicUsize::new(0xf731);
    unsafe {
        assert_eq!(atomic_nand(x.get_mut() as *mut usize, 0x137f), 0xf731);
    }
    assert_eq!(x.load(SeqCst), !(0xf731 & 0x137f));
}

#[test]
fn int_nand() {
    let mut x = AtomicIsize::new(0xf731);
    unsafe {
        assert_eq!(atomic_nand(x.get_mut() as *mut isize, 0x137f), 0xf731);
    }
    assert_eq!(x.load(SeqCst), !(0xf731 & 0x137f));
}
@Dushistov

This comment has been minimized.

Copy link
Contributor

Dushistov commented Feb 8, 2018

@Mark-Simulacrum

I unfortunately do not have an android either, and after spending multiple hours trying to > get an emulator setup I have decided to not try any more... the code below I believe is
the test for this problem today. If someone has a working android version, and can test it, > that would be great.

I've run it on android phone (cargo test --target=arm-linux-androideabi/rustc 1.25.0-nightly (bd98fe0c0 2018-02-06)),
all looks ok:

$ cargo test --target=arm-linux-androideabi
   Compiling android-atomic-13226 v0.1.0 (file:///home/evgeniy/projects/rust-infra/bugs/android-atomic-13226)
    Finished dev [unoptimized + debuginfo] target(s) in 0.88 secs
     Running target/arm-linux-androideabi/debug/deps/android_atomic_13226-a6d2087ae9be837c
Current directory: /home/evgeniy/projects/rust-infra/bugs/android-atomic-13226
/home/evgeniy/projects/rust-infra/bugs/android-atomic-13226/target/arm...26-a6d2087ae9be837c: 1 file pushed. 2.4 MB/s (5396036 bytes in 2.134s

running 2 tests
test uint_nand ... ok
test int_nand ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

ADB_SUCCESS!!!!!!!!!!!!!!
ADB_SUCCESS!!!!!!!!!!!!!!
[evgeniy@15inch android-atomic-13226]$ adb shell
shell@hammerhead:/ $ uname -a
Linux localhost 3.4.0-gcf10b7e #1 SMP PREEMPT Mon Sep 19 22:14:08 UTC 2016 armv7l
@Mark-Simulacrum

This comment has been minimized.

Copy link
Member

Mark-Simulacrum commented Feb 8, 2018

@alexcrichton looks like we may be able to enable these on Android.

@alexcrichton

This comment has been minimized.

Copy link
Member Author

alexcrichton commented Feb 8, 2018

Hurray!

@Mark-Simulacrum Mark-Simulacrum changed the title fetch_nand is not implemented for Atomic{Int,Uint} Tracking Issue: fetch_nand Feb 9, 2018

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Feb 9, 2018

Add fetch_nand.
cc rust-lang#13226 (the tracking issue)

kennytm added a commit to kennytm/rust that referenced this issue Feb 9, 2018

Rollup merge of rust-lang#48100 - Mark-Simulacrum:fetch-nand, r=alexc…
…richton

Add fetch_nand to atomics

I think this is all fine but I have little familiarity with the atomic code (or libcore in general) so I may have accidentally done something wrong here...

cc rust-lang#13226 (the tracking issue)

r? @alexcrichton

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Feb 9, 2018

Add fetch_nand.
cc rust-lang#13226 (the tracking issue)

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Feb 9, 2018

Add fetch_nand.
cc rust-lang#13226 (the tracking issue)

kennytm added a commit to kennytm/rust that referenced this issue Feb 10, 2018

Rollup merge of rust-lang#48100 - Mark-Simulacrum:fetch-nand, r=alexc…
…richton

Add fetch_nand to atomics

I think this is all fine but I have little familiarity with the atomic code (or libcore in general) so I may have accidentally done something wrong here...

cc rust-lang#13226 (the tracking issue)
@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Mar 17, 2018

It looks like the only issue (incorrect behavior on Android) has been resolved.

@rfcbot fcp merge

@rfcbot

This comment has been minimized.

Copy link

rfcbot commented Mar 17, 2018

Team member @SimonSapin has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot

This comment has been minimized.

Copy link

rfcbot commented Mar 18, 2018

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot

This comment has been minimized.

Copy link

rfcbot commented Mar 28, 2018

The final comment period is now complete.

bors added a commit that referenced this issue Apr 16, 2018

Auto merge of #49963 - llogiq:stabilize-13226, r=kennytm
stabilize fetch_nand

This closes #13226 and makes `Atomic*.fetch_nand` stable.

@bors bors closed this in #49963 Apr 16, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.