atomics A.1c: fix comptime signed fetch_min/max (was unsigned compare)
Adversarial review CRITICAL: the comptime VM's atomic_rmw min/max arm called @max/@min directly on Reg (=u64) values for SIGNED types, doing an UNSIGNED compare — so comptime fetch_min/max on negatives diverged from the runtime LLVM atomicrmw min/max (signed). Fix: reinterpret as i64 in the signed branch before comparing, bitcast back (mirrors the unsigned branch + the emit-side signedness). Closes the coverage gap that hid it: extend examples/1701 with signed min/max on a negative at BOTH comptime (#run) and runtime — they now agree (3 / -5). Suite green (716/0).
This commit is contained in:
@@ -1,8 +1,16 @@
|
|||||||
// Atomic($T) read-modify-write: fetch_add/sub/and/or/xor/min/max → LLVM atomicrmw.
|
// Atomic($T) read-modify-write: fetch_add/sub/and/or/xor/min/max → LLVM atomicrmw.
|
||||||
// Each returns the OLD value. Stream A (atomics) A.1. Single-thread.
|
// Each returns the OLD value. Stream A (atomics) A.1. Single-thread.
|
||||||
|
// Also covers signed min/max with NEGATIVES at BOTH comptime (#run) and runtime —
|
||||||
|
// they must agree (regression: comptime once did an unsigned compare).
|
||||||
#import "modules/std.sx";
|
#import "modules/std.sx";
|
||||||
#import "modules/std/atomic.sx";
|
#import "modules/std/atomic.sx";
|
||||||
|
|
||||||
|
// comptime (#run) signed min/max with a negative — must match runtime.
|
||||||
|
c_max :: () -> i64 { a := Atomic(i64).init(-5); _ := a.fetch_max(3, .seq_cst); return a.load(.seq_cst); }
|
||||||
|
c_min :: () -> i64 { a := Atomic(i64).init(-5); _ := a.fetch_min(3, .seq_cst); return a.load(.seq_cst); }
|
||||||
|
G_MAX :: #run c_max();
|
||||||
|
G_MIN :: #run c_min();
|
||||||
|
|
||||||
main :: () {
|
main :: () {
|
||||||
a := Atomic(i64).init(10);
|
a := Atomic(i64).init(10);
|
||||||
print("old add: {}\n", a.fetch_add(5, .seq_cst)); // returns 10, now 15
|
print("old add: {}\n", a.fetch_add(5, .seq_cst)); // returns 10, now 15
|
||||||
@@ -19,4 +27,13 @@ main :: () {
|
|||||||
print("old min: {}\n", m.fetch_min(8, .seq_cst)); // returns 20, now 8
|
print("old min: {}\n", m.fetch_min(8, .seq_cst)); // returns 20, now 8
|
||||||
print("old max: {}\n", m.fetch_max(15, .seq_cst)); // returns 8, now 15
|
print("old max: {}\n", m.fetch_max(15, .seq_cst)); // returns 8, now 15
|
||||||
print("now: {}\n", m.load(.seq_cst)); // 15
|
print("now: {}\n", m.load(.seq_cst)); // 15
|
||||||
|
|
||||||
|
// signed min/max with a negative — comptime (#run) and runtime must agree.
|
||||||
|
s := Atomic(i64).init(-5);
|
||||||
|
_ := s.fetch_max(3, .seq_cst);
|
||||||
|
print("runtime signed max(-5,3): {}\n", s.load(.seq_cst)); // 3
|
||||||
|
s.store(-5, .seq_cst);
|
||||||
|
_ := s.fetch_min(3, .seq_cst);
|
||||||
|
print("runtime signed min(-5,3): {}\n", s.load(.seq_cst)); // -5
|
||||||
|
print("comptime signed max(-5,3)={} min(-5,3)={}\n", G_MAX, G_MIN); // 3 / -5
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -8,3 +8,6 @@ now: 60
|
|||||||
old min: 20
|
old min: 20
|
||||||
old max: 8
|
old max: 8
|
||||||
now: 15
|
now: 15
|
||||||
|
runtime signed max(-5,3): 3
|
||||||
|
runtime signed min(-5,3): -5
|
||||||
|
comptime signed max(-5,3)=3 min(-5,3)=-5
|
||||||
|
|||||||
@@ -699,13 +699,17 @@ pub const Vm = struct {
|
|||||||
.@"or" => old | operand,
|
.@"or" => old | operand,
|
||||||
.xor => old ^ operand,
|
.xor => old ^ operand,
|
||||||
.min, .max => blk: {
|
.min, .max => blk: {
|
||||||
|
// `Reg` is u64, so `@max`/`@min` on it is an UNSIGNED
|
||||||
|
// compare. For a signed type, reinterpret as i64 first so
|
||||||
|
// a negative value loses to a positive one — matching LLVM
|
||||||
|
// `atomicrmw min`/`max` (signed) and the emit side.
|
||||||
const want_max = a.kind == .max;
|
const want_max = a.kind == .max;
|
||||||
if (table.isUnsignedInt(vty)) {
|
if (table.isUnsignedInt(vty)) {
|
||||||
const uo: u64 = @bitCast(old);
|
break :blk if (want_max) @max(old, operand) else @min(old, operand);
|
||||||
const up: u64 = @bitCast(operand);
|
|
||||||
break :blk @bitCast(if (want_max) @max(uo, up) else @min(uo, up));
|
|
||||||
}
|
}
|
||||||
break :blk if (want_max) @max(old, operand) else @min(old, operand);
|
const so: i64 = @bitCast(old);
|
||||||
|
const sp: i64 = @bitCast(operand);
|
||||||
|
break :blk @bitCast(if (want_max) @max(so, sp) else @min(so, sp));
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
try self.writeField(table, frame.get(a.ptr.index()), vty, new_val);
|
try self.writeField(table, frame.get(a.ptr.index()), vty, new_val);
|
||||||
|
|||||||
Reference in New Issue
Block a user