remove @fence (#21585)

closes #11650

doc/langref.html.in

@@ -4218,11 +4218,10 @@ pub fn print(self: *Writer, arg0: []const u8, arg1: i32) !void {
       {#header_close#}
 
       {#header_open|Atomics#}
-      <p>TODO: @fence()</p>
       <p>TODO: @atomic rmw</p>
       <p>TODO: builtin atomic memory ordering enum</p>
 
-      {#see_also|@atomicLoad|@atomicStore|@atomicRmw|@fence|@cmpxchgWeak|@cmpxchgStrong#}
+      {#see_also|@atomicLoad|@atomicStore|@atomicRmw|@cmpxchgWeak|@cmpxchgStrong#}
 
       {#header_close#}
 
@@ -4307,7 +4306,7 @@ comptime {
       an integer or an enum.
       </p>
       <p>{#syntax#}AtomicOrder{#endsyntax#} can be found with {#syntax#}@import("std").builtin.AtomicOrder{#endsyntax#}.</p>
-      {#see_also|@atomicStore|@atomicRmw|@fence|@cmpxchgWeak|@cmpxchgStrong#}
+      {#see_also|@atomicStore|@atomicRmw||@cmpxchgWeak|@cmpxchgStrong#}
       {#header_close#}
 
       {#header_open|@atomicRmw#}
@@ -4322,7 +4321,7 @@ comptime {
       </p>
       <p>{#syntax#}AtomicOrder{#endsyntax#} can be found with {#syntax#}@import("std").builtin.AtomicOrder{#endsyntax#}.</p>
       <p>{#syntax#}AtomicRmwOp{#endsyntax#} can be found with {#syntax#}@import("std").builtin.AtomicRmwOp{#endsyntax#}.</p>
-      {#see_also|@atomicStore|@atomicLoad|@fence|@cmpxchgWeak|@cmpxchgStrong#}
+      {#see_also|@atomicStore|@atomicLoad|@cmpxchgWeak|@cmpxchgStrong#}
       {#header_close#}
 
       {#header_open|@atomicStore#}
@@ -4335,7 +4334,7 @@ comptime {
       an integer or an enum.
       </p>
       <p>{#syntax#}AtomicOrder{#endsyntax#} can be found with {#syntax#}@import("std").builtin.AtomicOrder{#endsyntax#}.</p>
-      {#see_also|@atomicLoad|@atomicRmw|@fence|@cmpxchgWeak|@cmpxchgStrong#}
+      {#see_also|@atomicLoad|@atomicRmw|@cmpxchgWeak|@cmpxchgStrong#}
       {#header_close#}
 
       {#header_open|@bitCast#}
@@ -4568,7 +4567,7 @@ comptime {
       </p>
       <p>{#syntax#}@typeInfo(@TypeOf(ptr)).pointer.alignment{#endsyntax#} must be {#syntax#}>= @sizeOf(T).{#endsyntax#}</p>
       <p>{#syntax#}AtomicOrder{#endsyntax#} can be found with {#syntax#}@import("std").builtin.AtomicOrder{#endsyntax#}.</p>
-      {#see_also|@atomicStore|@atomicLoad|@atomicRmw|@fence|@cmpxchgWeak#}
+      {#see_also|@atomicStore|@atomicLoad|@atomicRmw|@cmpxchgWeak#}
       {#header_close#}
 
       {#header_open|@cmpxchgWeak#}
@@ -4600,7 +4599,7 @@ fn cmpxchgWeakButNotAtomic(comptime T: type, ptr: *T, expected_value: T, new_val
       </p>
       <p>{#syntax#}@typeInfo(@TypeOf(ptr)).pointer.alignment{#endsyntax#} must be {#syntax#}>= @sizeOf(T).{#endsyntax#}</p>
       <p>{#syntax#}AtomicOrder{#endsyntax#} can be found with {#syntax#}@import("std").builtin.AtomicOrder{#endsyntax#}.</p>
-      {#see_also|@atomicStore|@atomicLoad|@atomicRmw|@fence|@cmpxchgStrong#}
+      {#see_also|@atomicStore|@atomicLoad|@atomicRmw|@cmpxchgStrong#}
       {#header_close#}
 
       {#header_open|@compileError#}
@@ -4857,15 +4856,6 @@ fn cmpxchgWeakButNotAtomic(comptime T: type, ptr: *T, expected_value: T, new_val
       {#see_also|@export#}
       {#header_close#}
 
-      {#header_open|@fence#}
-      <pre>{#syntax#}@fence(order: AtomicOrder) void{#endsyntax#}</pre>
-      <p>
-      The {#syntax#}fence{#endsyntax#} function is used to introduce happens-before edges between operations.
-      </p>
-      <p>{#syntax#}AtomicOrder{#endsyntax#} can be found with {#syntax#}@import("std").builtin.AtomicOrder{#endsyntax#}.</p>
-      {#see_also|@atomicStore|@atomicLoad|@atomicRmw|@cmpxchgWeak|@cmpxchgStrong#}
-      {#header_close#}
-
       {#header_open|@field#}
       <pre>{#syntax#}@field(lhs: anytype, comptime field_name: []const u8) (field){#endsyntax#}</pre>
       <p>Performs field access by a compile-time string. Works on both fields and declarations.

lib/std/Thread/Futex.zig

@@ -794,9 +794,8 @@ const PosixImpl = struct {
         // - T1: bumps pending waiters (was reordered after the ptr == expect check)
         // - T1: goes to sleep and misses both the ptr change and T2's wake up
         //
-        // seq_cst as Acquire barrier to ensure the announcement happens before the ptr check below.
-        // seq_cst as shared modification order to form a happens-before edge with the fence(.seq_cst)+load() in wake().
-        var pending = bucket.pending.fetchAdd(1, .seq_cst);
+        // acquire barrier to ensure the announcement happens before the ptr check below.
+        var pending = bucket.pending.fetchAdd(1, .acquire);
         assert(pending < std.math.maxInt(usize));
 
         // If the wait gets cancelled, remove the pending count we previously added.
@@ -858,15 +857,8 @@ const PosixImpl = struct {
         //
         // What we really want here is a Release load, but that doesn't exist under the C11 memory model.
         // We could instead do `bucket.pending.fetchAdd(0, Release) == 0` which achieves effectively the same thing,
-        // but the RMW operation unconditionally marks the cache-line as modified for others causing unnecessary fetching/contention.
-        //
-        // Instead we opt to do a full-fence + load instead which avoids taking ownership of the cache-line.
-        // fence(seq_cst) effectively converts the ptr update to seq_cst and the pending load to seq_cst: creating a Store-Load barrier.
-        //
-        // The pending count increment in wait() must also now use seq_cst for the update + this pending load
-        // to be in the same modification order as our load isn't using release/acquire to guarantee it.
-        bucket.pending.fence(.seq_cst);
-        if (bucket.pending.load(.monotonic) == 0) {
+        // LLVM lowers the fetchAdd(0, .release) into an mfence+load which avoids gaining ownership of the cache-line.
+        if (bucket.pending.fetchAdd(0, .release) == 0) {
             return;
         }
 
@@ -979,15 +971,14 @@ test "broadcasting" {
         fn wait(self: *@This()) !void {
             // Decrement the counter.
             // Release ensures stuff before this barrier.wait() happens before the last one.
-            const count = self.count.fetchSub(1, .release);
+            // Acquire for the last counter ensures stuff before previous barrier.wait()s happened before it.
+            const count = self.count.fetchSub(1, .acq_rel);
             try testing.expect(count <= num_threads);
             try testing.expect(count > 0);
 
             // First counter to reach zero wakes all other threads.
-            // Acquire for the last counter ensures stuff before previous barrier.wait()s happened before it.
             // Release on futex update ensures stuff before all barrier.wait()'s happens before they all return.
             if (count - 1 == 0) {
-                _ = self.count.load(.acquire); // TODO: could be fence(acquire) if not for TSAN
                 self.futex.store(1, .release);
                 Futex.wake(&self.futex, num_threads - 1);
                 return;

lib/std/Thread/ResetEvent.zig

@@ -112,9 +112,9 @@ const FutexImpl = struct {
         // Try to set the state from `unset` to `waiting` to indicate
         // to the set() thread that others are blocked on the ResetEvent.
         // We avoid using any strict barriers until the end when we know the ResetEvent is set.
-        var state = self.state.load(.monotonic);
+        var state = self.state.load(.acquire);
         if (state == unset) {
-            state = self.state.cmpxchgStrong(state, waiting, .monotonic, .monotonic) orelse waiting;
+            state = self.state.cmpxchgStrong(state, waiting, .acquire, .acquire) orelse waiting;
         }
 
         // Wait until the ResetEvent is set since the state is waiting.
@@ -124,7 +124,7 @@ const FutexImpl = struct {
                 const wait_result = futex_deadline.wait(&self.state, waiting);
 
                 // Check if the ResetEvent was set before possibly reporting error.Timeout below.
-                state = self.state.load(.monotonic);
+                state = self.state.load(.acquire);
                 if (state != waiting) {
                     break;
                 }
@@ -133,9 +133,7 @@ const FutexImpl = struct {
             }
         }
 
-        // Acquire barrier ensures memory accesses before set() happen before we return.
         assert(state == is_set);
-        self.state.fence(.acquire);
     }
 
     fn set(self: *Impl) void {

lib/std/Thread/WaitGroup.zig

@@ -15,11 +15,10 @@ pub fn start(self: *WaitGroup) void {
 }
 
 pub fn finish(self: *WaitGroup) void {
-    const state = self.state.fetchSub(one_pending, .release);
+    const state = self.state.fetchSub(one_pending, .acq_rel);
     assert((state / one_pending) > 0);
 
     if (state == (one_pending | is_waiting)) {
-        self.state.fence(.acquire);
         self.event.set();
     }
 }

lib/std/atomic.zig

@@ -10,31 +10,7 @@ pub fn Value(comptime T: type) type {
             return .{ .raw = value };
         }
 
-        /// Perform an atomic fence which uses the atomic value as a hint for
-        /// the modification order. Use this when you want to imply a fence on
-        /// an atomic variable without necessarily performing a memory access.
-        pub inline fn fence(self: *Self, comptime order: AtomicOrder) void {
-            // LLVM's ThreadSanitizer doesn't support the normal fences so we specialize for it.
-            if (builtin.sanitize_thread) {
-                const tsan = struct {
-                    extern "c" fn __tsan_acquire(addr: *anyopaque) void;
-                    extern "c" fn __tsan_release(addr: *anyopaque) void;
-                };
-
-                const addr: *anyopaque = self;
-                return switch (order) {
-                    .unordered, .monotonic => @compileError(@tagName(order) ++ " only applies to atomic loads and stores"),
-                    .acquire => tsan.__tsan_acquire(addr),
-                    .release => tsan.__tsan_release(addr),
-                    .acq_rel, .seq_cst => {
-                        tsan.__tsan_acquire(addr);
-                        tsan.__tsan_release(addr);
-                    },
-                };
-            }
-
-            return @fence(order);
-        }
+        pub const fence = @compileError("@fence is deprecated, use other atomics to establish ordering");
 
         pub inline fn load(self: *const Self, comptime order: AtomicOrder) T {
             return @atomicLoad(T, &self.raw, order);
@@ -148,21 +124,19 @@ test Value {
         const RefCount = @This();
 
         fn ref(rc: *RefCount) void {
-            // No ordering necessary; just updating a counter.
+            // no synchronization necessary; just updating a counter.
             _ = rc.count.fetchAdd(1, .monotonic);
         }
 
         fn unref(rc: *RefCount) void {
-            // Release ensures code before unref() happens-before the
+            // release ensures code before unref() happens-before the
             // count is decremented as dropFn could be called by then.
             if (rc.count.fetchSub(1, .release) == 1) {
-                // acquire ensures count decrement and code before
-                // previous unrefs()s happens-before we call dropFn
-                // below.
-                // Another alternative is to use .acq_rel on the
-                // fetchSub count decrement but it's extra barrier in
-                // possibly hot path.
-                rc.count.fence(.acquire);
+                // seeing 1 in the counter means that other unref()s have happened,
+                // but it doesn't mean that uses before each unref() are visible.
+                // The load acquires the release-sequence created by previous unref()s
+                // in order to ensure visibility of uses before dropping.
+                _ = rc.count.load(.acquire);
                 (rc.dropFn)(rc);
             }
         }

lib/std/zig/AstGen.zig

@@ -2901,7 +2901,6 @@ fn addEnsureResult(gz: *GenZir, maybe_unused_result: Zir.Inst.Ref, statement: As
             .extended => switch (gz.astgen.instructions.items(.data)[@intFromEnum(inst)].extended.opcode) {
                 .breakpoint,
                 .disable_instrumentation,
-                .fence,
                 .set_float_mode,
                 .set_align_stack,
                 .branch_hint,
@@ -9307,15 +9306,6 @@ fn builtinCall(
             });
             return rvalue(gz, ri, result, node);
         },
-        .fence => {
-            const atomic_order_ty = try gz.addBuiltinValue(node, .atomic_order);
-            const order = try expr(gz, scope, .{ .rl = .{ .coerced_ty = atomic_order_ty } }, params[0]);
-            _ = try gz.addExtendedPayload(.fence, Zir.Inst.UnNode{
-                .node = gz.nodeIndexToRelative(node),
-                .operand = order,
-            });
-            return rvalue(gz, ri, .void_value, node);
-        },
         .set_float_mode => {
             const float_mode_ty = try gz.addBuiltinValue(node, .float_mode);
             const order = try expr(gz, scope, .{ .rl = .{ .coerced_ty = float_mode_ty } }, params[0]);

lib/std/zig/AstRlAnnotate.zig

@@ -908,7 +908,6 @@ fn builtinCall(astrl: *AstRlAnnotate, block: ?*Block, ri: ResultInfo, node: Ast.
         .c_include,
         .wasm_memory_size,
         .splat,
-        .fence,
         .set_float_mode,
         .set_align_stack,
         .type_info,

lib/std/zig/BuiltinFn.zig

@@ -48,7 +48,6 @@ pub const Tag = enum {
     error_cast,
     @"export",
     @"extern",
-    fence,
     field,
     field_parent_ptr,
     float_cast,
@@ -500,13 +499,6 @@ pub const list = list: {
                 .param_count = 2,
             },
         },
-        .{
-            "@fence",
-            .{
-                .tag = .fence,
-                .param_count = 1,
-            },
-        },
         .{
             "@field",
             .{

lib/std/zig/Zir.zig

@@ -1575,7 +1575,7 @@ pub const Inst = struct {
                 => false,
 
                 .extended => switch (data.extended.opcode) {
-                    .fence, .branch_hint, .breakpoint, .disable_instrumentation => true,
+                    .branch_hint, .breakpoint, .disable_instrumentation => true,
                     else => false,
                 },
             };
@@ -1979,9 +1979,6 @@ pub const Inst = struct {
         /// The `@prefetch` builtin.
         /// `operand` is payload index to `BinNode`.
         prefetch,
-        /// Implements the `@fence` builtin.
-        /// `operand` is payload index to `UnNode`.
-        fence,
         /// Implement builtin `@setFloatMode`.
         /// `operand` is payload index to `UnNode`.
         set_float_mode,
@@ -4014,7 +4011,6 @@ fn findDeclsInner(
                 .wasm_memory_size,
                 .wasm_memory_grow,
                 .prefetch,
-                .fence,
                 .set_float_mode,
                 .set_align_stack,
                 .error_cast,

lib/zig.h

@@ -3610,7 +3610,6 @@ typedef enum memory_order zig_memory_order;
 #define zig_atomicrmw_add_float zig_atomicrmw_add
 #undef  zig_atomicrmw_sub_float
 #define zig_atomicrmw_sub_float zig_atomicrmw_sub
-#define zig_fence(order) atomic_thread_fence(order)
 #elif defined(__GNUC__)
 typedef int zig_memory_order;
 #define zig_memory_order_relaxed __ATOMIC_RELAXED
@@ -3634,7 +3633,6 @@ typedef int zig_memory_order;
 #define    zig_atomic_load(res, obj,      order, Type, ReprType)       __atomic_load      (obj, &(res), order)
 #undef  zig_atomicrmw_xchg_float
 #define zig_atomicrmw_xchg_float zig_atomicrmw_xchg
-#define zig_fence(order) __atomic_thread_fence(order)
 #elif _MSC_VER && (_M_IX86 || _M_X64)
 #define zig_memory_order_relaxed 0
 #define zig_memory_order_acquire 2
@@ -3655,11 +3653,6 @@ typedef int zig_memory_order;
 #define  zig_atomicrmw_max(res, obj, arg, order, Type, ReprType) res = zig_msvc_atomicrmw_max_ ##Type(obj, arg)
 #define   zig_atomic_store(     obj, arg, order, Type, ReprType)       zig_msvc_atomic_store_  ##Type(obj, arg)
 #define    zig_atomic_load(res, obj,      order, Type, ReprType) res = zig_msvc_atomic_load_   ##order##_##Type(obj)
-#if _M_X64
-#define zig_fence(order) __faststorefence()
-#else
-#define zig_fence(order) zig_msvc_atomic_barrier()
-#endif
 /* TODO: _MSC_VER && (_M_ARM || _M_ARM64) */
 #else
 #define zig_memory_order_relaxed 0
@@ -3681,7 +3674,6 @@ typedef int zig_memory_order;
 #define  zig_atomicrmw_max(res, obj, arg, order, Type, ReprType) zig_atomics_unavailable
 #define   zig_atomic_store(     obj, arg, order, Type, ReprType) zig_atomics_unavailable
 #define    zig_atomic_load(res, obj,      order, Type, ReprType) zig_atomics_unavailable
-#define zig_fence(order) zig_fence_unavailable
 #endif
 
 #if _MSC_VER && (_M_IX86 || _M_X64)

src/Air.zig

@@ -734,10 +734,6 @@ pub const Inst = struct {
         cmpxchg_weak,
         /// Uses the `ty_pl` field with payload `Cmpxchg`.
         cmpxchg_strong,
-        /// Lowers to a memory fence instruction.
-        /// Result type is always void.
-        /// Uses the `fence` field.
-        fence,
         /// Atomically load from a pointer.
         /// Result type is the element type of the pointer.
         /// Uses the `atomic_load` field.
@@ -1066,7 +1062,6 @@ pub const Inst = struct {
             line: u32,
             column: u32,
         },
-        fence: std.builtin.AtomicOrder,
         atomic_load: struct {
             ptr: Ref,
             order: std.builtin.AtomicOrder,
@@ -1478,7 +1473,6 @@ pub fn typeOfIndex(air: *const Air, inst: Air.Inst.Index, ip: *const InternPool)
         .dbg_arg_inline,
         .store,
         .store_safe,
-        .fence,
         .atomic_store_unordered,
         .atomic_store_monotonic,
         .atomic_store_release,
@@ -1653,7 +1647,6 @@ pub fn mustLower(air: Air, inst: Air.Inst.Index, ip: *const InternPool) bool {
         .memcpy,
         .cmpxchg_weak,
         .cmpxchg_strong,
-        .fence,
         .atomic_store_unordered,
         .atomic_store_monotonic,
         .atomic_store_release,

src/Air/types_resolved.zig

@@ -416,7 +416,6 @@ fn checkBody(air: Air, body: []const Air.Inst.Index, zcu: *Zcu) bool {
             .work_item_id,
             .work_group_size,
             .work_group_id,
-            .fence,
             .dbg_stmt,
             .err_return_trace,
             .save_err_return_trace_index,