[libc-commits] [libc] [libc][GPU] Restore release/acquire ordering on the allocator bitfield (PR #238208)

[via libc-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libc

Author: Spencer Bryngelson (sbryngelson)

<details>
<summary>Changes</summary>

Fixes #<!-- -->238205.

`Slab::deallocate` clears a chunk's bit with a relaxed `fetch_and` and `Slab::allocate` claims it with a relaxed `fetch_or`. The release fence in `GuardPtr::unlock` runs after the clear, so the freeing thread's stores to the chunk are not ordered before the chunk can be claimed again. On MI300A a thread on another XCC can take the chunk, and the previous owner's dirty L2 lines are then written back over the new data.

This makes the clear a `RELEASE` and the claim an `ACQUIRE`, so `free` synchronizes with a later `malloc` of the same chunk. e9350dc had removed the equivalent fences.

Testing: the reproducer in #<!-- -->238205 on MI300A with `HSA_XNACK=1`, 10 launches of 20M allocations: damaged records in 10 of 10 launches before, 0 of 10 after. No slowdown was seen in a kernel-time micro-benchmark. `RELEASE` on the clear alone also cured it. There is no unit test: the failure needs several XCCs and heavy concurrent allocation. The libc GPU integration tests were not run; the library was built with AFAR 24.3.0's clang.


---
Full diff: https://github.com/llvm/llvm-project/pull/238208.diff


1 Files Affected:

- (modified) libc/src/__support/GPU/allocator.cpp (+2-2) 


``````````diff
diff --git a/libc/src/__support/GPU/allocator.cpp b/libc/src/__support/GPU/allocator.cpp
index 75b9ea73e609ca..ec28167269ef84 100644
--- a/libc/src/__support/GPU/allocator.cpp
+++ b/libc/src/__support/GPU/allocator.cpp
@@ -313,7 +313,7 @@ struct Slab {
         uint32_t before = 0;
         if (gpu::get_lane_id() == leader)
           before = cpp::AtomicRef(get_bitfield()[slot])
-                       .fetch_or(bitmask, cpp::MemoryOrder::RELAXED);
+                       .fetch_or(bitmask, cpp::MemoryOrder::ACQUIRE);
         before = gpu::shuffle(lane_mask, leader, before);
         if (~before & (1u << bit))
           result = ptr_from_index(index, chunk_size);
@@ -343,7 +343,7 @@ struct Slab {
     uint32_t bit = index % BITS_IN_WORD;
 
     cpp::AtomicRef(get_bitfield()[slot])
-        .fetch_and(~(1u << bit), cpp::MemoryOrder::RELAXED);
+        .fetch_and(~(1u << bit), cpp::MemoryOrder::RELEASE);
   }
 
   // The actual memory the slab will manage. All offsets are calculated at

``````````

</details>


https://github.com/llvm/llvm-project/pull/238208