Wrong results with atomic substraction (LLVM-50)

[rodrigorc]

Hi! I'm having issues with the result of atomic substractions.
I'm using the experimental Rust front-end, with a code such as this:

pub fn test() -> usize {
    use core::sync::atomic::Ordering::*;
    let mut x = core::sync::atomic::AtomicUsize::new(0);
    x.fetch_add(1, Relaxed);
    x.fetch_add(1, Relaxed);
    x.fetch_sub(1, Release);
    x.fetch_sub(1, Release);
    x.load(SeqCst)
}

I would expect it to return 0, but instead it returns 2.
The code generated seems ok, this is the LLVM-IR:

define internal fastcc void @_ZN6mytest4test17h53b3bfc678f42155E() unnamed_addr #0 {
start:
  %x = alloca %"core::sync::atomic::AtomicUsize", align 4
  %0 = bitcast %"core::sync::atomic::AtomicUsize"* %x to i8*
  call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull %0)
  %abi_cast.0..sroa_idx = getelementptr inbounds %"core::sync::atomic::AtomicUsize", %"core::sync::atomic::AtomicUsize"* %x, i32 0, i32 0, i32 0
  store i32 0, i32* %abi_cast.0..sroa_idx, align 4
  %1 = atomicrmw add i32* %abi_cast.0..sroa_idx, i32 1 monotonic
  %2 = atomicrmw add i32* %abi_cast.0..sroa_idx, i32 1 monotonic
  %3 = atomicrmw sub i32* %abi_cast.0..sroa_idx, i32 1 release
  %4 = atomicrmw sub i32* %abi_cast.0..sroa_idx, i32 1 release
  %5 = load atomic i32, i32* %abi_cast.0..sroa_idx seq_cst, align 4
  call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull %0)
  ret void
}

Looking at the intermediate values of x it goes this way:

let mut x = core::sync::atomic::AtomicUsize::new(0);
//x == 0
x.fetch_add(1, Relaxed);
//x == 1
x.fetch_add(1, Relaxed);
//x == 2
x.fetch_sub(1, Release);
//x == 4294967295 (that is -1)
x.fetch_sub(1, Release);
//x == 2 (I think this is kind of -4294967294)

I think it may be related to that comment in the freertos/portmacro.h file:

Warning: From the ISA docs: in some (unspecified) cases, the s32c1i instruction may return the
bitwise inverse of the old mem if the mem wasn't written. This doesn't seem to happen on the
ESP32 (portMUX assertions would fail).

It looks like this only happens when the instruction before the s32c1l instruction is a substraction. If I replace the fetch_sub(1) with a fetch_add(-1) it works as expected, with the following IR:
%4 = atomicrmw add i32* %abi_cast.0..sroa_idx, i32 -1 release

I looked at GCC with an equivalent C++ code using std::atomicand it uses add before s32c1l, even for substractions, never sub.

The conclusion I draw is that the LLVM atomicrmw sub should generate an add with negative value instead of a substraction.

[reitermarkus]

There is also a bug with AtomicI8, where fetch_add always returns 0:

let n = AtomicI8::new(0);

let n0 = n.fetch_add(1, SeqCst);
let n1 = n.fetch_add(1, SeqCst);

assert_eq!(n.load(SeqCst), 2);
assert_eq!(n0, 0);
assert_eq!(n1, 1);

which results in

thread '<unnamed>' panicked at 'assertion failed: `(left == right)`
  left: `0`,
 right: `1`', app/src/main.rs:8:3

fetch_add always returns 0 instead of the previous value, in this case n1 == 0, when you load n in the end, it has the right value, however.

[andreisfr]

Hi @rodrigorc, @reitermarkus! Please, verify issues with new Xtensa backend based on release 9.0.1. (default branch in repository)

[rodrigorc]

I've just tested my above sample code agains xtensa_release_9.0.1 branch and it works correctly now.
@reitermarkus' code also seems to work perfectly fine.

Thanks!

[reitermarkus]

I can confirm that the example I gave above is working as expected now.