Skip to content

remove @fence #21585

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.

Sign up for GitHub

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

Merged
andrewrk merged 3 commits into from
Oct 4, 2024
Merged

remove @fence #21585

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d2b5de7
remove `@fence`
Rexicon226 Sep 26, 2024
2cca8a1
remove `@fence` from the langref
Rexicon226 Sep 29, 2024
ff8ee4b
remove fence from `zig.h`
Rexicon226 Oct 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 6 additions & 16 deletions doc/langref.html.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4218,11 +4218,10 @@ pub fn print(self: *Writer, arg0: []const u8, arg1: i32) !void {
{#header_close#}

{#header_open|Atomics#}
<p>TODO: @fence()</p>
Copy link
Member

@andrewrk andrewrk Oct 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good thing I never bothered to write these docs 🙃

<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#}

Expand Down Expand Up @@ -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#}
Expand All @@ -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#}
Expand All @@ -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#}
Expand Down Expand Up @@ -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#}
Expand Down Expand Up @@ -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#}
Expand Down Expand Up @@ -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.
Expand Down
21 changes: 6 additions & 15 deletions lib/std/Thread/Futex.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down
8 changes: 3 additions & 5 deletions lib/std/Thread/ResetEvent.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
}
Expand All @@ -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 {
Expand Down
3 changes: 1 addition & 2 deletions lib/std/Thread/WaitGroup.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Expand Down
42 changes: 8 additions & 34 deletions lib/std/atomic.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
}
Expand Down
10 changes: 0 additions & 10 deletions lib/std/zig/AstGen.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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]);
Expand Down
1 change: 0 additions & 1 deletion lib/std/zig/AstRlAnnotate.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 0 additions & 8 deletions lib/std/zig/BuiltinFn.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ pub const Tag = enum {
error_cast,
@"export",
@"extern",
fence,
field,
field_parent_ptr,
float_cast,
Expand Down Expand Up @@ -500,13 +499,6 @@ pub const list = list: {
.param_count = 2,
},
},
.{
"@fence",
.{
.tag = .fence,
.param_count = 1,
},
},
.{
"@field",
.{
Expand Down
6 changes: 1 addition & 5 deletions lib/std/zig/Zir.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
};
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -4014,7 +4011,6 @@ fn findDeclsInner(
.wasm_memory_size,
.wasm_memory_grow,
.prefetch,
.fence,
.set_float_mode,
.set_align_stack,
.error_cast,
Expand Down
8 changes: 0 additions & 8 deletions lib/zig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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)
Expand Down
7 changes: 0 additions & 7 deletions src/Air.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion src/Air/types_resolved.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading