From c706d22f743fc1bac8001a195d228c14dcf3bd59 Mon Sep 17 00:00:00 2001 From: kalsprite Date: Wed, 19 Aug 2026 19:16:51 -0700 Subject: [PATCH 01/33] nbio: fix lost lake-up and stale op.l on windows --- core/nbio/impl_windows.odin | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/core/nbio/impl_windows.odin b/core/nbio/impl_windows.odin index 162092a80..d9e43f378 100644 --- a/core/nbio/impl_windows.odin +++ b/core/nbio/impl_windows.odin @@ -25,11 +25,6 @@ _Event_Loop :: struct { thread: win.HANDLE, completed: queue.Queue(^Operation), completed_oob: Multi_Producer_Single_Consumer, - state: enum { - Working, - Waking, - Sleeping, - }, } @(private="package") @@ -176,12 +171,8 @@ __tick :: proc(l: ^Event_Loop, timeout: time.Duration) -> (err: General_Error) { } if actual_timeout > 0 { - sync.atomic_store_explicit(&l.state, .Sleeping, .Release) - - // There could be a race condition where we go sleeping at the same time as things get queued - // and a wakeup isn't done because the state is not .Sleeping yet. - // So after sleeping we first check our queues. - + // Work may have been queued after the drain at the top of this tick, + // so check the queues once more before blocking. for { op := (^Operation)(mpsc_dequeue(&l.queue)) if op == nil { break } @@ -209,8 +200,6 @@ __tick :: proc(l: ^Event_Loop, timeout: time.Duration) -> (err: General_Error) { } } - sync.atomic_store_explicit(&l.state, .Working, .Relaxed) - if actual_timeout > 0 { // We may have just waited some time, lets update the current time. l.now = time.now() @@ -228,7 +217,7 @@ __tick :: proc(l: ^Event_Loop, timeout: time.Duration) -> (err: General_Error) { handle_completed(op) } else { op_l := op.l - for !mpsc_enqueue(&op.l.completed_oob, op) { + for !mpsc_enqueue(&op_l.completed_oob, op) { warn("oob queue filled up, QUEUE_SIZE may need increasing") _wake_up(op_l) win.SwitchToThread() @@ -770,10 +759,10 @@ _associate_socket :: proc(socket: Any_Socket, l: ^Event_Loop) -> Association_Err @(private="package") _wake_up :: proc(l: ^Event_Loop) { - _, exchanged := sync.atomic_compare_exchange_strong(&l.state, .Sleeping, .Waking) - if exchanged { - win.QueueUserAPC(proc "system" (Parameter: win.ULONG_PTR) {}, l.thread, 0) - } + // Unconditional: an APC queued before the loop enters its alertable wait stays + // pending and is delivered as soon as that wait begins. Queueing it only once the + // loop is already asleep drops every wake sent while it is still awake. + win.QueueUserAPC(proc "system" (Parameter: win.ULONG_PTR) {}, l.thread, 0) } @(private="package") From 2719a089ec943eafe7ffcaab2204640101f94310 Mon Sep 17 00:00:00 2001 From: kalsprite Date: Thu, 20 Aug 2026 20:43:08 -0700 Subject: [PATCH 02/33] recompute timeouts --- core/nbio/impl_windows.odin | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/core/nbio/impl_windows.odin b/core/nbio/impl_windows.odin index d9e43f378..2af88afb9 100644 --- a/core/nbio/impl_windows.odin +++ b/core/nbio/impl_windows.odin @@ -160,15 +160,7 @@ __tick :: proc(l: ^Event_Loop, timeout: time.Duration) -> (err: General_Error) { if pool.num_outstanding(&l.operation_pool) == 0 { return nil } - actual_timeout := win.INFINITE - if queue.len(l.completed) > 0 || mpsc_count(&l.completed_oob) > 0 { - actual_timeout = 0 - } else if timeout >= 0 { - actual_timeout = win.DWORD(timeout / time.Millisecond) - } - if nt, ok := next_timeout.?; ok { - actual_timeout = min(actual_timeout, win.DWORD(nt / time.Millisecond)) - } + actual_timeout := compute_timeout(l, timeout, next_timeout) if actual_timeout > 0 { // Work may have been queued after the drain at the top of this tick, @@ -184,6 +176,10 @@ __tick :: proc(l: ^Event_Loop, timeout: time.Duration) -> (err: General_Error) { if op == nil { break } handle_completed(op) } + + // The drains can add timeouts, and `timeout_exec` only puts those in + // `l.timeouts` without posting anything + actual_timeout = compute_timeout(l, timeout, check_timeouts(l)) } for { @@ -237,6 +233,19 @@ __tick :: proc(l: ^Event_Loop, timeout: time.Duration) -> (err: General_Error) { return nil + compute_timeout :: proc(l: ^Event_Loop, timeout: time.Duration, next_timeout: Maybe(time.Duration)) -> win.DWORD { + actual: win.DWORD = win.INFINITE + if queue.len(l.completed) > 0 || mpsc_count(&l.completed_oob) > 0 { + actual = 0 + } else if timeout >= 0 { + actual = win.DWORD(timeout / time.Millisecond) + } + if nt, ok := next_timeout.?; ok { + actual = min(actual, win.DWORD(nt / time.Millisecond)) + } + return actual + } + check_timeouts :: proc(l: ^Event_Loop) -> (expires: Maybe(time.Duration)) { curr := l.now From ae56bf54564bd88018bc0cf4984dd8194d6a637c Mon Sep 17 00:00:00 2001 From: kalsprite Date: Thu, 20 Aug 2026 23:04:26 -0700 Subject: [PATCH 03/33] nbio(windows): clear the OVERLAPPED when arming a poll; nbio(windows): tear down poll waits before their event; nbio(windows): clear the socket event record on poll completion --- core/nbio/impl_windows.odin | 39 ++++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/core/nbio/impl_windows.odin b/core/nbio/impl_windows.odin index 2af88afb9..a8c99e05b 100644 --- a/core/nbio/impl_windows.odin +++ b/core/nbio/impl_windows.odin @@ -676,7 +676,7 @@ _remove :: proc(target: ^Operation) { switch target.type { case .Poll: - win.UnregisterWaitEx(target.poll._impl.wait_handle, nil) + win.UnregisterWaitEx(target.poll._impl.wait_handle, win.INVALID_HANDLE_VALUE) target.poll._impl.wait_handle = nil ok := win.PostQueuedCompletionStatus( @@ -1500,9 +1500,14 @@ sendfile_callback :: proc(op: ^Operation) -> Op_Result { return .Done } +// Bit indices into `WSANETWORKEVENTS.iErrorCode`, corresponding to the `FD_*` masks. +FD_READ_BIT :: 0 +FD_WRITE_BIT :: 1 + @(require_results) poll_exec :: proc(op: ^Operation) -> Op_Result { assert(op.type == .Poll) + op._impl.over = {} // Operations are recycled, clear stale state from a previous use. events: i32 = win.FD_CLOSE switch op.poll.event { @@ -1565,12 +1570,40 @@ poll_exec :: proc(op: ^Operation) -> Op_Result { poll_callback :: proc(op: ^Operation) { assert(op.type == .Poll) + // Clear the socket's internal network event record, and find out what actually + // fired. Without this the record stays set after an event is reported, so the next + // `WSAEventSelect` on that socket signals its event object immediately from the + // stale record. That completes a poll for a readiness that never happened, and the + // send/recv the caller then makes fails with WOULDBLOCK. if op._impl.over.hEvent != nil { - win.WSACloseEvent(op._impl.over.hEvent) + nev: win.WSANETWORKEVENTS + sk := win.SOCKET(net.any_socket_to_socket(op.poll.socket)) + if win.WSAEnumNetworkEvents(sk, op._impl.over.hEvent, &nev) == 0 { + bit: uint + switch op.poll.event { + case .Receive: bit = FD_READ_BIT + case .Send: bit = FD_WRITE_BIT + } + + // Only downgrade a result that is still `Ready`; `wait_callback` may have + // already set `.Timeout`. + if op.poll.result == nil && nev.lNetworkEvents & (i32(1) << bit) != 0 && nev.iErrorCode[bit] != 0 { + op.poll.result = .Error + } + } } + // Tear down in the reverse order of `poll_exec`: stop `wait_callback` from running + // before the event it waits on goes away. `INVALID_HANDLE_VALUE` waits for an + // in-flight callback to return, so it can't touch `op` after it is recycled. if op.poll._impl.wait_handle != nil { - win.UnregisterWaitEx(op.poll._impl.wait_handle, nil) + win.UnregisterWaitEx(op.poll._impl.wait_handle, win.INVALID_HANDLE_VALUE) + op.poll._impl.wait_handle = nil + } + + if op._impl.over.hEvent != nil { + win.WSACloseEvent(op._impl.over.hEvent) + op._impl.over.hEvent = nil } if op.poll.result != nil { From bcf2dcd2f0afeac70bb28ba88f0299c1ae0b11f9 Mon Sep 17 00:00:00 2001 From: kalsprite Date: Thu, 20 Aug 2026 23:39:21 -0700 Subject: [PATCH 04/33] nbio(windows): don't complete synchronously-failed overlapped ops twice --- core/nbio/impl_windows.odin | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/core/nbio/impl_windows.odin b/core/nbio/impl_windows.odin index a8c99e05b..de16a5a69 100644 --- a/core/nbio/impl_windows.odin +++ b/core/nbio/impl_windows.odin @@ -923,6 +923,9 @@ accept_exec :: proc(op: ^Operation) -> Op_Result { return .Pending } else if op._impl.over.Internal == nil { op.accept.err = net._accept_error() + } else { + link_timeout(op, op.accept.expires) + return .Pending } } @@ -1024,6 +1027,9 @@ dial_exec :: proc(op: ^Operation) -> (result: Op_Result) { return .Pending } else if op._impl.over.Internal == nil { op.dial.err = net._dial_error() + } else { + link_timeout(op, op.dial.expires) + return .Pending } } @@ -1083,6 +1089,13 @@ read_exec :: proc(op: ^Operation) -> Op_Result { return .Pending } op.read.err = FS_Error(err) + } else { + // The read completed synchronously with a failure status. `FILE_SKIP_COMPLETION_PORT_ON_SUCCESS` + // only suppresses the completion packet on success, so one is still queued for + // this. Returning `.Done` here would complete the operation a second time, on an + // Operation that has already been recycled into the pool. + link_timeout(op, op.read.expires) + return .Pending } } @@ -1157,6 +1170,9 @@ write_exec :: proc(op: ^Operation) -> Op_Result { return .Pending } op.write.err = FS_Error(err) + } else { + link_timeout(op, op.write.expires) + return .Pending } } @@ -1250,6 +1266,9 @@ recv_exec :: proc(op: ^Operation) -> Op_Result { case TCP_Socket: op.recv.err = net._tcp_recv_error() case UDP_Socket: op.recv.err = net._udp_recv_error() } + } else { + link_timeout(op, op.recv.expires) + return .Pending } } @@ -1368,6 +1387,9 @@ send_exec :: proc(op: ^Operation) -> Op_Result { case TCP_Socket: op.send.err = net._tcp_send_error() case UDP_Socket: op.send.err = net._udp_send_error() } + } else { + link_timeout(op, op.send.expires) + return .Pending } } @@ -1457,6 +1479,9 @@ sendfile_exec :: proc(op: ^Operation) -> Op_Result { return .Pending } else if op._impl.over.Internal == nil { op.sendfile.err = net._tcp_send_error() + } else { + link_timeout(op, op.sendfile.expires) + return .Pending } } From 6e690afaf1d455cf29c7997839f2a39e9d226a42 Mon Sep 17 00:00:00 2001 From: kalsprite Date: Fri, 21 Aug 2026 00:09:43 -0700 Subject: [PATCH 05/33] revert wake change --- core/nbio/impl_windows.odin | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/core/nbio/impl_windows.odin b/core/nbio/impl_windows.odin index de16a5a69..e1e0f2ebc 100644 --- a/core/nbio/impl_windows.odin +++ b/core/nbio/impl_windows.odin @@ -25,6 +25,11 @@ _Event_Loop :: struct { thread: win.HANDLE, completed: queue.Queue(^Operation), completed_oob: Multi_Producer_Single_Consumer, + state: enum { + Working, + Waking, + Sleeping, + }, } @(private="package") @@ -163,8 +168,11 @@ __tick :: proc(l: ^Event_Loop, timeout: time.Duration) -> (err: General_Error) { actual_timeout := compute_timeout(l, timeout, next_timeout) if actual_timeout > 0 { - // Work may have been queued after the drain at the top of this tick, - // so check the queues once more before blocking. + sync.atomic_store_explicit(&l.state, .Sleeping, .Release) + + // There could be a race condition where we go sleeping at the same time as things get queued + // and a wakeup isn't done because the state is not .Sleeping yet. + // So after sleeping we first check our queues. for { op := (^Operation)(mpsc_dequeue(&l.queue)) if op == nil { break } @@ -196,6 +204,8 @@ __tick :: proc(l: ^Event_Loop, timeout: time.Duration) -> (err: General_Error) { } } + sync.atomic_store_explicit(&l.state, .Working, .Relaxed) + if actual_timeout > 0 { // We may have just waited some time, lets update the current time. l.now = time.now() @@ -768,10 +778,10 @@ _associate_socket :: proc(socket: Any_Socket, l: ^Event_Loop) -> Association_Err @(private="package") _wake_up :: proc(l: ^Event_Loop) { - // Unconditional: an APC queued before the loop enters its alertable wait stays - // pending and is delivered as soon as that wait begins. Queueing it only once the - // loop is already asleep drops every wake sent while it is still awake. - win.QueueUserAPC(proc "system" (Parameter: win.ULONG_PTR) {}, l.thread, 0) + _, exchanged := sync.atomic_compare_exchange_strong(&l.state, .Sleeping, .Waking) + if exchanged { + win.QueueUserAPC(proc "system" (Parameter: win.ULONG_PTR) {}, l.thread, 0) + } } @(private="package") From d2da33235d3a63b418097bc42812ada9696f7672 Mon Sep 17 00:00:00 2001 From: kalsprite Date: Fri, 21 Aug 2026 09:18:57 -0700 Subject: [PATCH 06/33] nbio(windows): handle queued work in the tick that was woken for it; nbio(tests): let wake_up tolerate a tick that returns without progress --- core/nbio/impl_windows.odin | 14 ++++++++++++++ tests/core/nbio/nbio.odin | 7 +++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/core/nbio/impl_windows.odin b/core/nbio/impl_windows.odin index e1e0f2ebc..7357f7c18 100644 --- a/core/nbio/impl_windows.odin +++ b/core/nbio/impl_windows.odin @@ -241,6 +241,20 @@ __tick :: proc(l: ^Event_Loop, timeout: time.Duration) -> (err: General_Error) { actual_timeout = 0 } + // A wake, or another loop routing a completion to us, can leave work queued. + // Handle it here instead of waiting for the caller to tick again. + for { + op := (^Operation)(mpsc_dequeue(&l.queue)) + if op == nil { break } + _exec(op) + } + + for { + op := (^Operation)(mpsc_dequeue(&l.completed_oob)) + if op == nil { break } + handle_completed(op) + } + return nil compute_timeout :: proc(l: ^Event_Loop, timeout: time.Duration, next_timeout: Maybe(time.Duration)) -> win.DWORD { diff --git a/tests/core/nbio/nbio.odin b/tests/core/nbio/nbio.odin index 6121d1ac7..5d43fd814 100644 --- a/tests/core/nbio/nbio.odin +++ b/tests/core/nbio/nbio.odin @@ -244,8 +244,11 @@ wake_up :: proc(t: ^testing.T) { }, context) defer thread.destroy(thr) - // Should block forever until the thread calling wake_up will make it return. - ev(t, nbio.tick(), nil) + // A tick can return without progress; loop until the wake is observed. + // A lost wake would block here forever and trip the fail timeout. + for !hit { + ev(t, nbio.tick(), nil) + } e(t, hit) nbio.remove(accept) From 1b4e14fbc549e532fe5e2fc614ab66502e849c93 Mon Sep 17 00:00:00 2001 From: kalsprite Date: Fri, 21 Aug 2026 20:43:33 -0700 Subject: [PATCH 07/33] restore fail timeout --- tests/core/nbio/net.odin | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/nbio/net.odin b/tests/core/nbio/net.odin index 77e44f73b..f0b8226c6 100644 --- a/tests/core/nbio/net.odin +++ b/tests/core/nbio/net.odin @@ -245,7 +245,7 @@ And it tests big send/recv buffers being handled properly. @(test) poll :: proc(t: ^testing.T) { if event_loop_guard(t) { -// testing.set_fail_timeout(t, time.Minute) + testing.set_fail_timeout(t, time.Minute) can_recv: bool From 7bb55ba45b4c1d463e1f096fcde35e15052c5fa9 Mon Sep 17 00:00:00 2001 From: kalsprite Date: Fri, 21 Aug 2026 21:31:49 -0700 Subject: [PATCH 08/33] nbio(windows): poll sockets through AFD --- core/nbio/impl_windows.odin | 224 +++++++++++++++++++++--------------- core/nbio/ops.odin | 12 ++ core/sys/windows/ntdll.odin | 13 +++ tests/core/nbio/remove.odin | 17 ++- 4 files changed, 167 insertions(+), 99 deletions(-) diff --git a/core/nbio/impl_windows.odin b/core/nbio/impl_windows.odin index 7357f7c18..4d202620e 100644 --- a/core/nbio/impl_windows.odin +++ b/core/nbio/impl_windows.odin @@ -19,6 +19,38 @@ import win "core:sys/windows" @(private="package") _FULLY_SUPPORTED :: true +// Poll is driven by AFD, the socket driver underneath winsock. +// `WSAEventSelect` is edge triggered (`FD_WRITE` is only recorded again after a +// send fails with WOULDBLOCK) and neither `select` nor `WSAPoll` reports send +// buffer space, so neither can give the level triggered readiness `poll` promises. +// AFD also completes on the IOCP, which makes a poll an ordinary overlapped operation. +IOCTL_AFD_POLL :: 0x00012024 +SIO_BASE_HANDLE :: win.DWORD(0x48000022) + +AFD_POLL_RECEIVE :: 0x0001 +AFD_POLL_RECEIVE_EXPEDITED :: 0x0002 +AFD_POLL_SEND :: 0x0004 +AFD_POLL_DISCONNECT :: 0x0008 +AFD_POLL_ABORT :: 0x0010 +AFD_POLL_LOCAL_CLOSE :: 0x0020 +AFD_POLL_ACCEPT :: 0x0080 +AFD_POLL_CONNECT_FAIL :: 0x0100 + +AFD_Poll_Handle_Info :: struct { + handle: win.HANDLE, + events: win.ULONG, + status: win.NTSTATUS, +} + +AFD_Poll_Info :: struct { + timeout: i64, + number_of_handles: win.ULONG, + exclusive: win.ULONG, + handles: [1]AFD_Poll_Handle_Info, +} + +afd_device_name := [?]u16{'\\','D','e','v','i','c','e','\\','A','f','d','\\','E','n','d','p','o','i','n','t'} + @(private="package") _Event_Loop :: struct { timeouts: avl.Tree(^Operation), @@ -88,7 +120,7 @@ _Timeout :: struct { @(private="package") _Poll :: struct { - wait_handle: win.HANDLE, + info: AFD_Poll_Info, } @(private="package") @@ -699,19 +731,6 @@ _remove :: proc(target: ^Operation) { target._impl.timeout = (^Operation)(REMOVED) switch target.type { - case .Poll: - win.UnregisterWaitEx(target.poll._impl.wait_handle, win.INVALID_HANDLE_VALUE) - target.poll._impl.wait_handle = nil - - ok := win.PostQueuedCompletionStatus( - g.iocp, - 0, - 0, - &target._impl.over, - ) - ensure(ok == true, "unexpected PostQueuedCompletionStatus error") - return - case .Timeout: if avl.remove_value(&target.l.timeouts, target) { debug("removed timeout directly") @@ -727,6 +746,17 @@ _remove :: proc(target: ^Operation) { // Synchronous ops, picked up in handler. return + case .Poll: + // The poll may have completed already, with its completion queued but not yet + // handled, `NOT_FOUND` is expected rather than exceptional. + if !win.CancelIoEx(g.afd, &target._impl.over) { + #partial switch win.System_Error(win.GetLastError()) { + case .NOT_FOUND: + // nop + case: assert(false, "unexpected CancelIoEx error") + } + } + case .Accept, .Dial, .Read, .Recv, .Send, .Write, .Send_File: if is_pending(target._impl.over) { handle := operation_handle(target) @@ -824,6 +854,7 @@ g: struct{ mu: sync.Mutex, refs: int, iocp: win.HANDLE, + afd: win.HANDLE, err: General_Error, } @@ -839,6 +870,36 @@ g_ref :: proc() -> General_Error { if g.iocp == nil { g.err = General_Error(win.GetLastError()) } + + if g.err != nil { return g.err } + + // A handle on the socket driver, used to poll sockets for readiness. + iosb: win.IO_STATUS_BLOCK + status := win.NtCreateFile( + &g.afd, + win.SYNCHRONIZE, + &{ + Length = size_of(win.OBJECT_ATTRIBUTES), + ObjectName = &{ + Length = u16(len(afd_device_name)*2), + MaximumLength = u16(len(afd_device_name)*2), + Buffer = raw_data(afd_device_name[:]), + }, + }, + &iosb, + nil, + 0, + win.FILE_SHARE_READ|win.FILE_SHARE_WRITE, + win.FILE_OPEN, + 0, + nil, + 0, + ) + if syserr := win.System_Error(win.RtlNtStatusToDosError(status)); syserr != .SUCCESS { + g.err = General_Error(syserr) + } else if win.CreateIoCompletionPort(g.afd, g.iocp, 0, 0) != g.iocp { + g.err = General_Error(win.GetLastError()) + } } sync.atomic_add(&g.refs, 1) @@ -850,6 +911,7 @@ g_unref :: proc() { sync.guard(&g.mu) if sync.atomic_sub(&g.refs, 1) == 1 { + if g.afd != nil { win.CloseHandle(g.afd) } win.CloseHandle(g.iocp) g.err = nil } @@ -872,7 +934,7 @@ operation_handle :: proc(op: ^Operation) -> win.HANDLE { case .Recv: return win.HANDLE(uintptr(net.any_socket_to_socket(op.recv.socket))) case .Send: return win.HANDLE(uintptr(net.any_socket_to_socket(op.send.socket))) case .Send_File: return win.HANDLE(uintptr(net.any_socket_to_socket(op.sendfile.socket))) - case .Poll: return win.HANDLE(uintptr(net.any_socket_to_socket(op.poll.socket))) + case .Poll: return g.afd case .Stat: return win.HANDLE(uintptr(op.stat.handle)) case .Timeout, .Open, ._Splice, ._Link_Timeout, ._Remove, .None: @@ -1549,120 +1611,94 @@ sendfile_callback :: proc(op: ^Operation) -> Op_Result { return .Done } -// Bit indices into `WSANETWORKEVENTS.iErrorCode`, corresponding to the `FD_*` masks. -FD_READ_BIT :: 0 -FD_WRITE_BIT :: 1 - @(require_results) poll_exec :: proc(op: ^Operation) -> Op_Result { assert(op.type == .Poll) op._impl.over = {} // Operations are recycled, clear stale state from a previous use. - events: i32 = win.FD_CLOSE + events: win.ULONG = AFD_POLL_ABORT|AFD_POLL_DISCONNECT|AFD_POLL_LOCAL_CLOSE|AFD_POLL_CONNECT_FAIL switch op.poll.event { - case .Send: events |= win.FD_WRITE|win.FD_CONNECT - case .Receive: events |= win.FD_READ|win.FD_ACCEPT + case .Receive: events |= AFD_POLL_RECEIVE|AFD_POLL_RECEIVE_EXPEDITED|AFD_POLL_ACCEPT + case .Send: events |= AFD_POLL_SEND case: op.poll.result = .Invalid_Argument return .Done } - op._impl.over.hEvent = win.WSACreateEvent() - if win.WSAEventSelect( + // AFD needs the socket underneath any layered service providers. + base: win.SOCKET + bytes: win.DWORD + if win.WSAIoctl( win.SOCKET(net.any_socket_to_socket(op.poll.socket)), - op._impl.over.hEvent, - events, + SIO_BASE_HANDLE, + nil, 0, + &base, size_of(base), + &bytes, nil, nil, ) != 0 { - #partial switch win.System_Error(win.GetLastError()) { + #partial switch win.System_Error(win.WSAGetLastError()) { case .WSAEINVAL, .WSAENOTSOCK: op.poll.result = .Invalid_Argument case: op.poll.result = .Error } return .Done } - timeout := win.INFINITE + // A negative timeout is relative, in 100ns units. + timeout := max(i64) if op.poll.expires != {} { diff := max(0, time.diff(op.l.now, op.poll.expires)) - timeout = win.DWORD(diff / time.Millisecond) + timeout = -i64(diff / 100) } - ok := win.RegisterWaitForSingleObject( - &op.poll._impl.wait_handle, - op._impl.over.hEvent, - wait_callback, - op, - timeout, - win.WT_EXECUTEINWAITTHREAD|win.WT_EXECUTEONLYONCE, + op.poll._impl.info = { + timeout = timeout, + number_of_handles = 1, + handles = {{handle = win.HANDLE(uintptr(base)), events = events}}, + } + + // The OVERLAPPED doubles as the IO_STATUS_BLOCK, their first two fields line up. + status := win.NtDeviceIoControlFile( + g.afd, + nil, + nil, + &op._impl.over, + win.PIO_STATUS_BLOCK(rawptr(&op._impl.over)), + IOCTL_AFD_POLL, + &op.poll._impl.info, + size_of(AFD_Poll_Info), + &op.poll._impl.info, + size_of(AFD_Poll_Info), ) - ensure(ok == true, "unexpected RegisterWaitForSingleObject error") - return .Pending - - wait_callback :: proc "system" (lpParameter: win.PVOID, TimerOrWaitFired: win.BOOLEAN) { - op := (^Operation)(lpParameter) - assert_contextless(op.type == .Poll) - - if TimerOrWaitFired { - op.poll.result = .Timeout - } - - ok := win.PostQueuedCompletionStatus( - g.iocp, - 0, - 0, - &op._impl.over, - ) - ensure_contextless(ok == true, "unexpected PostQueuedCompletionStatus error") + // The AFD handle is not set to skip completion on success, so a completion is + // queued even when this finishes synchronously. + #partial switch win.System_Error(win.RtlNtStatusToDosError(status)) { + case .SUCCESS, .IO_PENDING: + return .Pending + case: + op.poll.result = .Error + return .Done } } poll_callback :: proc(op: ^Operation) { assert(op.type == .Poll) - // Clear the socket's internal network event record, and find out what actually - // fired. Without this the record stays set after an event is reported, so the next - // `WSAEventSelect` on that socket signals its event object immediately from the - // stale record. That completes a poll for a readiness that never happened, and the - // send/recv the caller then makes fails with WOULDBLOCK. - if op._impl.over.hEvent != nil { - nev: win.WSANETWORKEVENTS - sk := win.SOCKET(net.any_socket_to_socket(op.poll.socket)) - if win.WSAEnumNetworkEvents(sk, op._impl.over.hEvent, &nev) == 0 { - bit: uint - switch op.poll.event { - case .Receive: bit = FD_READ_BIT - case .Send: bit = FD_WRITE_BIT - } - - // Only downgrade a result that is still `Ready`; `wait_callback` may have - // already set `.Timeout`. - if op.poll.result == nil && nev.lNetworkEvents & (i32(1) << bit) != 0 && nev.iErrorCode[bit] != 0 { - op.poll.result = .Error - } - } - } - - // Tear down in the reverse order of `poll_exec`: stop `wait_callback` from running - // before the event it waits on goes away. `INVALID_HANDLE_VALUE` waits for an - // in-flight callback to return, so it can't touch `op` after it is recycled. - if op.poll._impl.wait_handle != nil { - win.UnregisterWaitEx(op.poll._impl.wait_handle, win.INVALID_HANDLE_VALUE) - op.poll._impl.wait_handle = nil - } - - if op._impl.over.hEvent != nil { - win.WSACloseEvent(op._impl.over.hEvent) - op._impl.over.hEvent = nil - } - if op.poll.result != nil { return } - _, err := get_result(op._impl.over) - #partial switch err { - case .SUCCESS: - case: + // AFD reports a timeout by coming back with no handles. + if op.poll._impl.info.number_of_handles == 0 { + op.poll.result = .Timeout + return + } + + if _, err := get_result(op._impl.over); err != .SUCCESS { + op.poll.result = .Error + return + } + + if op.poll._impl.info.handles[0].events & (AFD_POLL_ABORT|AFD_POLL_CONNECT_FAIL) != 0 { op.poll.result = .Error } } diff --git a/core/nbio/ops.odin b/core/nbio/ops.odin index 382dca747..6af266e72 100644 --- a/core/nbio/ops.odin +++ b/core/nbio/ops.odin @@ -1626,6 +1626,9 @@ Poll a socket for readiness. NOTE: this is provided to help with "legacy" APIs that require polling behavior. If you can avoid it and use the other procs in this package, do so. +NOTE: on Windows only one poll per socket is delivered, a second poll on the same +socket does not complete. + Any user data can be set on the returned operation's `user_data` field. Polymorphic variants for type safe user data are available under `poll_poly`, `poll_poly2`, and `poll_poly3`. @@ -1656,6 +1659,9 @@ Poll a socket for readiness. NOTE: this is provided to help with "legacy" APIs that require polling behavior. If you can avoid it and use the other procs in this package, do so. +NOTE: on Windows only one poll per socket is delivered, a second poll on the same +socket does not complete. + This procedure uses polymorphism for type safe user data up to a certain size. Inputs: @@ -1690,6 +1696,9 @@ Poll a socket for readiness. NOTE: this is provided to help with "legacy" APIs that require polling behavior. If you can avoid it and use the other procs in this package, do so. +NOTE: on Windows only one poll per socket is delivered, a second poll on the same +socket does not complete. + This procedure uses polymorphism for type safe user data up to a certain size. Inputs: @@ -1725,6 +1734,9 @@ Poll a socket for readiness. NOTE: this is provided to help with "legacy" APIs that require polling behavior. If you can avoid it and use the other procs in this package, do so. +NOTE: on Windows only one poll per socket is delivered, a second poll on the same +socket does not complete. + This procedure uses polymorphism for type safe user data up to a certain size. Inputs: diff --git a/core/sys/windows/ntdll.odin b/core/sys/windows/ntdll.odin index 41deaa1c4..a78228642 100644 --- a/core/sys/windows/ntdll.odin +++ b/core/sys/windows/ntdll.odin @@ -7,6 +7,19 @@ foreign import ntdll_lib "system:ntdll.lib" foreign ntdll_lib { RtlGetVersion :: proc(lpVersionInformation: ^OSVERSIONINFOEXW) -> NTSTATUS --- + NtDeviceIoControlFile :: proc( + FileHandle: HANDLE, + Event: HANDLE, + ApcRoutine: PIO_APC_ROUTINE, + ApcContext: rawptr, + IoStatusBlock: PIO_STATUS_BLOCK, + IoControlCode: ULONG, + InputBuffer: rawptr, + InputBufferLength: ULONG, + OutputBuffer: rawptr, + OutputBufferLength: ULONG, + ) -> NTSTATUS --- + NtQueryInformationProcess :: proc( ProcessHandle: HANDLE, diff --git a/tests/core/nbio/remove.odin b/tests/core/nbio/remove.odin index 063c2cf58..37d32c5e1 100644 --- a/tests/core/nbio/remove.odin +++ b/tests/core/nbio/remove.odin @@ -212,13 +212,18 @@ remove_multiple_poll :: proc(t: ^testing.T) { if event_loop_guard(t) { testing.set_fail_timeout(t, time.Minute) - sock, ep := open_next_available_local_port(t) - defer nbio.close(sock) + // Two sockets rather than two polls on one socket: only one poll per socket is + // delivered on Windows, and what this tests is removal, not that. + removed_sock, removed_ep := open_next_available_local_port(t) + defer nbio.close(removed_sock) + + kept_sock, kept_ep := open_next_available_local_port(t) + defer nbio.close(kept_sock) hit: bool - first := nbio.poll(sock, .Receive, on_poll) - nbio.poll_poly2(sock, .Receive, t, &hit, on_poll2) + first := nbio.poll(removed_sock, .Receive, on_poll) + nbio.poll_poly2(kept_sock, .Receive, t, &hit, on_poll2) on_poll :: proc(op: ^nbio.Operation) { log.error("shouldn't be called") @@ -235,7 +240,9 @@ remove_multiple_poll :: proc(t: ^testing.T) { ev(t, nbio.tick(0), nil) - nbio.dial_poly(ep, t, on_dial) + // Make both readable, the removed poll must still not fire. + nbio.dial_poly(removed_ep, t, on_dial) + nbio.dial_poly(kept_ep, t, on_dial) on_dial :: proc(op: ^nbio.Operation, t: ^testing.T) { ev(t, op.dial.err, nil) From d29b2cadbd1bb3709f789f02e8ddfbc9078b98f0 Mon Sep 17 00:00:00 2001 From: kalsprite Date: Fri, 21 Aug 2026 21:58:40 -0700 Subject: [PATCH 09/33] nbio(tests): fill the socket until sending blocks --- tests/core/nbio/net.odin | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/core/nbio/net.odin b/tests/core/nbio/net.odin index f0b8226c6..544fb6f58 100644 --- a/tests/core/nbio/net.odin +++ b/tests/core/nbio/net.odin @@ -302,13 +302,25 @@ poll :: proc(t: ^testing.T) { on_poll1 :: proc(op: ^nbio.Operation, t: ^testing.T, can_recv: ^bool) { ev(t, op.poll.result, nil) - // Send 4 GB of data, which in my experience causes a Would_Block error because we filled up the internal buffer. + // Fill the socket until sending actually blocks. How much that takes depends + // on the machine's socket buffers, so keep sending rather than assuming a + // fixed amount does it. Nothing is reading yet, so this terminates. buf, mem_err := make([]byte, mem.Gigabyte*4, context.temp_allocator) ev(t, mem_err, nil) // Use `core:net` as example external code that doesn't care about the event loop. net.set_blocking(op.poll.socket, false) - n, send_err := net.send(op.poll.socket, buf) + + n: int + send_err: net.Network_Error + for _ in 0..<16 { + sent: int + sent, send_err = net.send(op.poll.socket, buf) + n += sent + if send_err != nil { + break + } + } ev(t, send_err, net.TCP_Send_Error.Would_Block) log.debugf("blocking after %M", n) From fe52f23b502c7ca7132dd16a13674685e5865679 Mon Sep 17 00:00:00 2001 From: kalsprite Date: Fri, 21 Aug 2026 15:25:18 -0700 Subject: [PATCH 10/33] tests/core/nbio/remove.odin --- tests/core/nbio/remove.odin | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/core/nbio/remove.odin b/tests/core/nbio/remove.odin index 37d32c5e1..fe7884c28 100644 --- a/tests/core/nbio/remove.odin +++ b/tests/core/nbio/remove.odin @@ -58,7 +58,12 @@ immediate_remove_of_sendfile :: proc(t: ^testing.T) { } on_recv :: proc(op: ^nbio.Operation, t: ^testing.T) { - ev(t, op.recv.err, nil) + // The server cancelled a sendfile that had already put bytes on the wire and + // then closed, which ends the connection with a reset rather than gracefully + // often enough that both have to be accepted here. + if op.recv.err != nil { + ev(t, op.recv.err, net.TCP_Recv_Error.Connection_Closed) + } nbio.close(op.recv.socket.(net.TCP_Socket)) } @@ -126,7 +131,12 @@ immediate_remove_of_sendfile_without_stat :: proc(t: ^testing.T) { } on_recv :: proc(op: ^nbio.Operation, t: ^testing.T) { - ev(t, op.recv.err, nil) + // The server cancelled a sendfile that had already put bytes on the wire and + // then closed, which ends the connection with a reset rather than gracefully + // often enough that both have to be accepted here. + if op.recv.err != nil { + ev(t, op.recv.err, net.TCP_Recv_Error.Connection_Closed) + } nbio.close(op.recv.socket.(net.TCP_Socket)) } From b69e61983a93f492259337a30d4c1053a81865dd Mon Sep 17 00:00:00 2001 From: gingerBill Date: Mon, 24 Aug 2026 19:52:56 +0100 Subject: [PATCH 11/33] Mock out the data structures for working on a CFG for `asm` --- src/check_asm.cpp | 145 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 113 insertions(+), 32 deletions(-) diff --git a/src/check_asm.cpp b/src/check_asm.cpp index f8fa94d0f..1f2517210 100644 --- a/src/check_asm.cpp +++ b/src/check_asm.cpp @@ -1,3 +1,64 @@ +struct AsmBlock { + i32 first, last; + Array succs; + u16 in_defs; + u16 out_defs; + PtrSet in_params; + PtrSet out_params; + bool reachable; +}; + +struct AsmInstructionFacts { + AstAsmInstruction *node; + String name; + + u16 gen_regs; + u16 read_regs; + + Array gen_params; + Array read_params; + + bool is_control; + bool is_conditional; + bool is_terminal; + + Entity *branch_target; + i32 block_id; +}; + + +struct AsmMnemonicAccumulator { + u16 defined_regs; + PtrSet defined_params; + + // Union of registers implicitly clobbered by matched forms (for redundant-#clobber hints). + u16 implicit_clobbered_regs; + u16 explicitly_produced_regs; + u16 stale_outputs; + + bool straight_line; + + // Whether the most-recently-checked instruction terminates straight-line flow. + // Reset to false at every label (a label starts a fresh straight-line region whose + // tail we haven't seen yet). Consulted after the loop for #diverging templates. + bool last_is_terminal; + + // Did the template contain any instructions at all? An empty diverging body can't diverge. + bool saw_any_instructions; + + // Related to #align_stack + // any call/branch (CONTROL) or memory effect that could require the stack + // to be realigned. If none occurred, #align_stack is redundant. + bool saw_call_or_mem; + + // Purity test + bool can_be_pure; + char const *impure_reason; + Ast * impure_reason_node; + + PtrMap instruction_facts; +}; + // Bit-width the operand's Odin type occupies in a register/immediate slot. // Integers/floats/bools/pointers -> their size; #simd -> total vector width. 0 if unknown. gb_internal i32 check_asm_operand_bit_width(Type *type) { @@ -1042,36 +1103,6 @@ gb_internal CheckMnemomicResult check_mnemonic_name(AsmCtx *asm_ctx, AstAsmInstr return CheckMnemomic_Invalid; } -struct AsmMnemonicAccumulator { - u16 defined_regs; - PtrSet defined_params; - - // Union of registers implicitly clobbered by matched forms (for redundant-#clobber hints). - u16 implicit_clobbered_regs; - u16 explicitly_produced_regs; - u16 stale_outputs; - - bool straight_line; - - // Whether the most-recently-checked instruction terminates straight-line flow. - // Reset to false at every label (a label starts a fresh straight-line region whose - // tail we haven't seen yet). Consulted after the loop for #diverging templates. - bool last_is_terminal; - - // Did the template contain any instructions at all? An empty diverging body can't diverge. - bool saw_any_instructions; - - // Related to #align_stack - // any call/branch (CONTROL) or memory effect that could require the stack - // to be realigned. If none occurred, #align_stack is redundant. - bool saw_call_or_mem; - - // Purity test - bool can_be_pure; - char const *impure_reason; - Ast * impure_reason_node; -}; - gb_internal bool check_asm_instr_targets_internal_label(AstAsmInstruction *instr) { bool saw_label = false; for (Ast *op : instr->operands) { @@ -1149,6 +1180,29 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm name = asm_ctx->pseudo_mnemonic_strings[pseudo_mnemonic]; } + AsmInstructionFacts facts = {}; + facts.node = instr; + facts.name = name; + facts.gen_params.allocator = heap_allocator(); + facts.read_params.allocator = heap_allocator(); + + defer ({ + for (auto const &op : operands) { + if (op.expr->kind != Ast_AsmLabelDecl) { + continue; + } + Entity *e = op.expr->AsmLabelDecl.name->Ident.entity; + if (e == nullptr || e->kind != Entity_Label) { + continue; + } + facts.branch_target = e; + break; + } + + map_set(&asm_acc->instruction_facts, instr, facts); + }); + + bool is_pseudo = pseudo_mnemonic != 0; int target_explicit_count = is_pseudo ? alias.nargs : -1; @@ -1546,6 +1600,8 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm // Handle clobbering from mnemonic auto clobber = clobber_forms[valid_form_index]; + facts.read_regs = cast(u16)clobber.implicit_rd & asm_ctx->CLOBBER_REGS_NAMED; + // NOTE(bill): reads_mem/writes_mem are per-FORM capability bits. // A form with an r/m slot (e.g. add r/m32, imm32) carries them even // when the operand resolved to a register, e.g. `add x, 123`. @@ -1704,7 +1760,26 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm produced |= synth; explicit_writes |= synth; } - asm_acc->defined_regs |= produced | pinned_param_writes; + + facts.gen_regs = produced | pinned_param_writes; + asm_acc->defined_regs |= facts.gen_regs; + + for_array(i, operands) { + int slot = user_operand_target_index(cast(int)i); + if (slot < 0) { + continue; + } + Entity *pe = entity_of_node(operands[i].expr); + if (pe == nullptr || pe->kind != Entity_Variable) { + continue; + } + if (cast(u16)clobber.read & (1u << slot)) { + array_add(&facts.read_params, pe); + } + if (cast(u16)clobber.written & (1u << slot)) { + array_add(&facts.gen_params, pe); + } + } if (asm_acc->straight_line) { // NOTE(bill): check for read-before-write (use of undefined value) @@ -1769,6 +1844,10 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm bool conditional = clobber.is_conditional(); asm_acc->last_is_terminal = halt || (control && !conditional); + facts.is_control = control; + facts.is_conditional = conditional; + facts.is_terminal = halt || (control && !conditional); + if (control) { // A branch/call inside the template means subsequent instructions may be reached // out of textual order; stop trusting the linear def model past this point. @@ -1809,7 +1888,6 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm asm_acc->impure_reason_node = instr->name; } } - return; } @@ -2466,6 +2544,9 @@ gb_internal void check_asm_template(AsmCtx *asm_ctx, CheckerContext *ctx, Entity ptr_set_init(&asm_acc.defined_params); defer (ptr_set_destroy(&asm_acc.defined_params)); + map_init(&asm_acc.instruction_facts); + defer (map_destroy(&asm_acc.instruction_facts)); + // Physical registers known to hold a defined value at the current point in the // straight-line instruction stream. Seeded with input-pinned registers (they From 322f59dae002f1791a6a6b61695daf205f644c6e Mon Sep 17 00:00:00 2001 From: gingerBill Date: Mon, 24 Aug 2026 21:49:32 +0100 Subject: [PATCH 12/33] Implement a CFG for the assembler to improve the soundness of the checks --- .../riscv/tablegen/cpp-compiler/cpp-gen.odin | 15 + .../x86/tablegen/cpp-compiler/cpp-gen.odin | 39 ++ src/asm_tables_amd64.cpp | 35 ++ src/asm_tables_riscv.cpp | 11 + src/check_asm.cpp | 261 ++------- src/check_asm_cfg.cpp | 541 ++++++++++++++++++ src/entity.cpp | 3 + 7 files changed, 681 insertions(+), 224 deletions(-) create mode 100644 src/check_asm_cfg.cpp diff --git a/core/rexcode/isa/riscv/tablegen/cpp-compiler/cpp-gen.odin b/core/rexcode/isa/riscv/tablegen/cpp-compiler/cpp-gen.odin index dbd9a4ca5..f4f70d9be 100644 --- a/core/rexcode/isa/riscv/tablegen/cpp-compiler/cpp-gen.odin +++ b/core/rexcode/isa/riscv/tablegen/cpp-compiler/cpp-gen.odin @@ -821,6 +821,21 @@ main :: proc() { } """) + strings.write_string(&sb, """ + bool is_self_zeroing_idiom(u16 m) const { + switch (m) { + case M_XOR: + case M_SUB: + case M_SUBW: + case M_SLT: + case M_SLTU: + case M_ANDN: + return true; + } + return false; + } + """) + strings.write_string(&sb, "\n};\n") strings.write_string(&sb, "\n\n\n") diff --git a/core/rexcode/isa/x86/tablegen/cpp-compiler/cpp-gen.odin b/core/rexcode/isa/x86/tablegen/cpp-compiler/cpp-gen.odin index 084f0acff..d64c6b2e4 100644 --- a/core/rexcode/isa/x86/tablegen/cpp-compiler/cpp-gen.odin +++ b/core/rexcode/isa/x86/tablegen/cpp-compiler/cpp-gen.odin @@ -824,6 +824,45 @@ main :: proc() { } """) + strings.write_string(&sb, """ + bool is_self_zeroing_idiom(u16 m) const { + switch (m) { + // integer xor / sub: x ^ x == 0, x - x == 0 + case M_XOR: + case M_SUB: + + // SSE/AVX bitwise xor of a register with itself + case M_PXOR: + case M_XORPS: + case M_XORPD: + case M_VPXOR: + case M_VXORPS: + case M_VXORPD: + + // packed integer subtract: psub x, x == 0 + case M_PSUBB: + case M_PSUBW: + case M_PSUBD: + case M_PSUBQ: + case M_VPSUBB: + case M_VPSUBW: + case M_VPSUBD: + case M_VPSUBQ: + + // andnot of a value with itself: (~x) & x == 0 + case M_ANDN: // BMI1 GPR: andn dst, a, a + case M_ANDNPS: + case M_ANDNPD: + case M_PANDN: + case M_VANDNPS: + case M_VANDNPD: + case M_VPANDN: + return true; + } + return false; + } + """) + strings.write_string(&sb, "\n};\n") strings.write_string(&sb, "\n\n\n") diff --git a/src/asm_tables_amd64.cpp b/src/asm_tables_amd64.cpp index 1302c7c33..4fd0d03e6 100644 --- a/src/asm_tables_amd64.cpp +++ b/src/asm_tables_amd64.cpp @@ -813,6 +813,41 @@ struct Asm_amd64 { break; } return {AsmOperandConstraint_None, -1}; + } bool is_self_zeroing_idiom(u16 m) const { + switch (m) { + // integer xor / sub: x ^ x == 0, x - x == 0 + case M_XOR: + case M_SUB: + + // SSE/AVX bitwise xor of a register with itself + case M_PXOR: + case M_XORPS: + case M_XORPD: + case M_VPXOR: + case M_VXORPS: + case M_VXORPD: + + // packed integer subtract: psub x, x == 0 + case M_PSUBB: + case M_PSUBW: + case M_PSUBD: + case M_PSUBQ: + case M_VPSUBB: + case M_VPSUBW: + case M_VPSUBD: + case M_VPSUBQ: + + // andnot of a value with itself: (~x) & x == 0 + case M_ANDN: // BMI1 GPR: andn dst, a, a + case M_ANDNPS: + case M_ANDNPD: + case M_PANDN: + case M_VANDNPS: + case M_VANDNPD: + case M_VPANDN: + return true; + } + return false; } }; diff --git a/src/asm_tables_riscv.cpp b/src/asm_tables_riscv.cpp index 85714c37e..62fddbda9 100644 --- a/src/asm_tables_riscv.cpp +++ b/src/asm_tables_riscv.cpp @@ -755,6 +755,17 @@ struct Asm_riscv { break; } return {AsmOperandConstraint_None, -1}; + } bool is_self_zeroing_idiom(u16 m) const { + switch (m) { + case M_XOR: + case M_SUB: + case M_SUBW: + case M_SLT: + case M_SLTU: + case M_ANDN: + return true; + } + return false; } }; diff --git a/src/check_asm.cpp b/src/check_asm.cpp index 1f2517210..e57839569 100644 --- a/src/check_asm.cpp +++ b/src/check_asm.cpp @@ -1,63 +1,4 @@ -struct AsmBlock { - i32 first, last; - Array succs; - u16 in_defs; - u16 out_defs; - PtrSet in_params; - PtrSet out_params; - bool reachable; -}; - -struct AsmInstructionFacts { - AstAsmInstruction *node; - String name; - - u16 gen_regs; - u16 read_regs; - - Array gen_params; - Array read_params; - - bool is_control; - bool is_conditional; - bool is_terminal; - - Entity *branch_target; - i32 block_id; -}; - - -struct AsmMnemonicAccumulator { - u16 defined_regs; - PtrSet defined_params; - - // Union of registers implicitly clobbered by matched forms (for redundant-#clobber hints). - u16 implicit_clobbered_regs; - u16 explicitly_produced_regs; - u16 stale_outputs; - - bool straight_line; - - // Whether the most-recently-checked instruction terminates straight-line flow. - // Reset to false at every label (a label starts a fresh straight-line region whose - // tail we haven't seen yet). Consulted after the loop for #diverging templates. - bool last_is_terminal; - - // Did the template contain any instructions at all? An empty diverging body can't diverge. - bool saw_any_instructions; - - // Related to #align_stack - // any call/branch (CONTROL) or memory effect that could require the stack - // to be realigned. If none occurred, #align_stack is redundant. - bool saw_call_or_mem; - - // Purity test - bool can_be_pure; - char const *impure_reason; - Ast * impure_reason_node; - - PtrMap instruction_facts; -}; +#include "check_asm_cfg.cpp" // Bit-width the operand's Odin type occupies in a register/immediate slot. // Integers/floats/bools/pointers -> their size; #simd -> total vector width. 0 if unknown. @@ -1647,51 +1588,6 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm } } - if (asm_acc->straight_line) { - u16 wants = cast(u16)clobber.implicit_rd & asm_ctx->CLOBBER_REGS_NAMED; - u16 undefined = wants & ~asm_acc->defined_regs; - - for (u16 bit = 1; bit != 0; bit <<= 1) { - if ((undefined & bit) == 0) { - continue; - } - char const *rname = asm_ctx->clobber_reg_bit_name(bit); - - String owner = {}; - char const *role = nullptr; - for (auto const &ed : tmpl_entity->AsmTemplate.decls) { - if (ed.pin.len == 0 || ed.entity == nullptr) { - continue; - } - if (asm_ctx->clobber_bit_for_reg_name(ed.pin) != bit) { - continue; - } - if (ed.param_group == AsmTemplateEntityDeclParamGroup_Output && ed.tie < 0) { - owner = ed.entity->token.string; - role = "output"; - break; - } - if (ed.param_group == AsmTemplateEntityDeclParamGroup_Scratch && ed.view_of < 0) { - owner = ed.entity->token.string; - role = "scratch"; - break; - } - } - - if (role != nullptr) { - error(instr->name, - "'%.*s' implicitly reads %%%s, which is bound to the %s parameter '%.*s', " - "but nothing has written %%%s yet; write to it (e.g. into '%.*s') before this instruction", - LIT(name), rname, role, LIT(owner), rname, LIT(owner)); - } else { - error(instr->name, - "'%.*s' implicitly reads %%%s, but nothing in this template produces a value for it; " - "pin an input parameter to %%%s, or write %%%s before this instruction", - LIT(name), rname, rname, rname); - } - } - } - u16 produced = cast(u16)clobber.implicit_wr & asm_ctx->CLOBBER_REGS_NAMED; u16 explicit_writes = 0; @@ -1762,7 +1658,28 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm } facts.gen_regs = produced | pinned_param_writes; - asm_acc->defined_regs |= facts.gen_regs; + + // NOTE(bill): mnemonics such as `xor r, r` / `sub r, r` act as zeroing the destination + // independent of its prior value: the read is architecturally dead, so it must not count as a use. + bool self_zeroing = false; + if (asm_ctx->is_self_zeroing_idiom(cast(u16)mnemonic) && operands.count >= 2) { + Entity *e0 = entity_of_node(operands[0].expr); + bool all_same = (e0 != nullptr); + for (isize k = 1; all_same && k < operands.count; k++) { + all_same = entity_of_node(operands[k].expr) == e0; + } + // also treat literal %reg == %reg as self-zeroing (no entity, compare reg bits) + if (!all_same && operands[0].expr->kind == Ast_AsmRegister) { + u16 b0 = asm_ctx->clobber_bit_for_reg_name(operands[0].expr->AsmRegister.name.string); + all_same = b0 != 0; + for (isize k = 1; all_same && k < operands.count; k++) { + all_same = operands[k].expr->kind == Ast_AsmRegister && + asm_ctx->clobber_bit_for_reg_name(operands[k].expr->AsmRegister.name.string) == b0; + } + } + self_zeroing = all_same; + } + for_array(i, operands) { int slot = user_operand_target_index(cast(int)i); @@ -1773,7 +1690,8 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm if (pe == nullptr || pe->kind != Entity_Variable) { continue; } - if (cast(u16)clobber.read & (1u << slot)) { + + if (!self_zeroing && (cast(u16)clobber.read & (1u << slot))) { array_add(&facts.read_params, pe); } if (cast(u16)clobber.written & (1u << slot)) { @@ -1781,40 +1699,6 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm } } - if (asm_acc->straight_line) { - // NOTE(bill): check for read-before-write (use of undefined value) - // Only really meaningful in straight-line code, so a label above means a branch - // could have defined the value out of the textual order - for_array(i, operands) { - auto const &op = operands[i]; - int slot = user_operand_target_index(cast(int)i); - if (slot < 0 || (cast(u16)clobber.read & (1u << slot)) == 0) { - continue; // not a read slot of this form - } - Entity *pe = entity_of_node(op.expr); - if (pe == nullptr || pe->kind != Entity_Variable) { - continue; // literal %reg / immediate / memory, not a tracked param - } - if (!ptr_set_exists(&asm_acc->defined_params, pe)) { - error(op.expr, "'%.*s' reads '%.*s' before it is assigned; its initial value is undefined", - LIT(name), LIT(pe->token.string)); - ptr_set_add(&asm_acc->defined_params, pe); // warn once per param - } - } - } - - // NOTE(bill): record the instruction's parameter writes - for_array(i, operands) { - int slot = user_operand_target_index(cast(int)i); - if (slot < 0 || (cast(u16)clobber.written & (1u << slot)) == 0) { - continue; - } - Entity *pe = entity_of_node(operands[i].expr); - if (pe != nullptr && pe->kind == Entity_Variable) { - ptr_set_add(&asm_acc->defined_params, pe); - } - } - { // Registers this form clobbers implicitly (RDTSC->RAX:RDX, etc.), for the // redundant-#clobber hint. Union across the template; pinned regs excluded @@ -1842,17 +1726,10 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm bool halt = clobber.has_halt(); // A conditional branch reads a flag and can fall through -> not terminal. bool conditional = clobber.is_conditional(); - asm_acc->last_is_terminal = halt || (control && !conditional); facts.is_control = control; facts.is_conditional = conditional; facts.is_terminal = halt || (control && !conditional); - - if (control) { - // A branch/call inside the template means subsequent instructions may be reached - // out of textual order; stop trusting the linear def model past this point. - asm_acc->straight_line = false; - } } asm_ctx->clobber_implicit_regs(&tmpl_entity->AsmTemplate.clobber_registers_set, produced); @@ -2426,7 +2303,6 @@ gb_internal void check_asm_template(AsmCtx *asm_ctx, CheckerContext *ctx, Entity entity->type = type; - bool is_volatile = false; bool is_align_stack = false; bool is_pure_annotated = false; @@ -2541,44 +2417,10 @@ gb_internal void check_asm_template(AsmCtx *asm_ctx, CheckerContext *ctx, Entity } AsmMnemonicAccumulator asm_acc = {}; - ptr_set_init(&asm_acc.defined_params); - defer (ptr_set_destroy(&asm_acc.defined_params)); - map_init(&asm_acc.instruction_facts); defer (map_destroy(&asm_acc.instruction_facts)); - - // Physical registers known to hold a defined value at the current point in the - // straight-line instruction stream. Seeded with input-pinned registers (they - // carry their argument at entry); grows as instructions write registers. - for (auto const &ed : ate->decls) { - if (ed.no_init) { - ptr_set_add(&asm_acc.defined_params, ed.entity); - } - switch (ed.param_group) { - case AsmTemplateEntityDeclParamGroup_Input: - if (ed.pin.len != 0) { - // Only inputs (and the input half of a tie, which is Input-group) hold a - // value at entry. Output/scratch pins start undefined and become defined - // when an instruction writes them. - asm_acc.defined_regs |= asm_ctx->clobber_bit_for_reg_name(ed.pin); - } - ptr_set_add(&asm_acc.defined_params, ed.entity); - break; - case AsmTemplateEntityDeclParamGroup_Output: - if (ed.tie >= 0) { - ptr_set_add(&asm_acc.defined_params, ed.entity); - } - break; - } - } - - // Linear "written earlier in the text" is only a sound proxy for "produced at - // runtime" while control flow is straight-line. The first label is a potential - // jump target / back-edge, after which a read can precede its textual def; from - // there on we stop emitting the implicit-read diagnostic. - asm_acc.straight_line = true; - asm_acc.can_be_pure = true; + asm_acc.can_be_pure = true; // collect label decls for (Ast *instruction_ : at->instructions) { @@ -2678,10 +2520,6 @@ gb_internal void check_asm_template(AsmCtx *asm_ctx, CheckerContext *ctx, Entity case_end; case_ast_node(label, AsmLabelDecl, instruction_); - asm_acc.straight_line = false; - // A new straight-line region begins here; its tail is unseen, - // so the previous instruction's terminality no longer describes the body's end. - asm_acc.last_is_terminal = false; if (previous_prefix != 0) { error(previous_prefix_instr, "A prefix must be immediately followed by an instruction, but a label declaration was found"); previous_prefix = 0; @@ -2780,32 +2618,17 @@ gb_internal void check_asm_template(AsmCtx *asm_ctx, CheckerContext *ctx, Entity error(previous_prefix_instr, "A prefix must be immediately followed by an instruction, but the template ended"); } - for (auto const &ed : ate->decls) { - if (ed.param_group != AsmTemplateEntityDeclParamGroup_Output) { - continue; - } - if (ed.tie >= 0) { - continue; - } - if (ed.no_init) { - continue; - } - if (!asm_acc.straight_line) { - continue; - } + // NOTE(bill, 2026-08-24): Construct a control-flow graph (CFG) from the instructions + // to do further analysis which is not possible with an conservative straight-line approximation + // Using a CFG is a much sounder approach for calculating: + // * reads before writes + // * divergence + // * unreachable code - bool written = false; - if (ed.pin.len != 0) { - u16 bit = asm_ctx->clobber_bit_for_reg_name(ed.pin); - written = (bit != 0) && (asm_acc.defined_regs & bit) != 0; - } else { - written = ptr_set_exists(&asm_acc.defined_params, ed.entity); - } - - if (!written) { - error(ed.entity->token, "'asm' output parameter '%.*s' is never assigned to in this template, thus its value is undefined", LIT(ed.entity->token.string)); - } - } + AsmCfg cfg = {}; + defer (asm_cfg_destroy(&cfg)); + check_asm_cfg_build(d->init_expr, &asm_acc, &cfg); + check_asm_cfg_analyse(asm_ctx, ctx, entity, &cfg, &asm_acc); bool vet_unused = false; @@ -2866,7 +2689,7 @@ gb_internal void check_asm_template(AsmCtx *asm_ctx, CheckerContext *ctx, Entity } } if (ed.tie > 0) { - // TODO(bill): Handle this edge case + // TODO(bill): Handle this edge case? continue; } @@ -2893,16 +2716,6 @@ gb_internal void check_asm_template(AsmCtx *asm_ctx, CheckerContext *ctx, Entity "that would require the stack to be realigned"); } - if (type->Proc.diverging) { - if (!asm_acc.saw_any_instructions) { - error(entity->token, "This asm template is declared as diverging (-> !) but its body is empty and cannot diverge"); - } else if (!asm_acc.last_is_terminal) { - error(entity->token, - "This asm template is declared diverging (-> !) but its final instruction can fall through; " - "end it with an unconditional jump, return, or halt"); - } - } - { bool declared_effects = entity->AsmTemplate.is_volatile || entity->AsmTemplate.clobber_memory || diff --git a/src/check_asm_cfg.cpp b/src/check_asm_cfg.cpp new file mode 100644 index 000000000..a02994ca6 --- /dev/null +++ b/src/check_asm_cfg.cpp @@ -0,0 +1,541 @@ +struct AsmBlock { + i32 first, last; + Array succs; + u16 in_defs; + u16 out_defs; + PtrSet in_params; + PtrSet out_params; + bool reachable; +}; + +struct AsmInstructionFacts { + AstAsmInstruction *node; + String name; + + u16 gen_regs; + u16 read_regs; + + Array gen_params; + Array read_params; + + bool is_control; + bool is_conditional; + bool is_terminal; + + Entity *branch_target; + i32 block_id; +}; + + +struct AsmMnemonicAccumulator { + // Union of registers implicitly clobbered by matched forms (for redundant-#clobber hints). + u16 implicit_clobbered_regs; + u16 explicitly_produced_regs; + u16 stale_outputs; + + bool saw_any_instructions; // NOTE(bill): An empty diverging body cannot diverge. + + // NOTE(bill): Related to #align_stack + // any call/branch (CONTROL) or memory effect that could require the stack + // to be realigned. If none occurred, #align_stack is redundant. + bool saw_call_or_mem; + + // Purity test + bool can_be_pure; + char const *impure_reason; + Ast * impure_reason_node; + + PtrMap instruction_facts; +}; + +struct AsmCfg { + Array insts; // program-order (only for fact-carrying instrs) + Array blocks; + PtrMap label_block; // key: Entity_Label* +}; + +gb_internal void asm_cfg_destroy(AsmCfg *cfg) { + for (auto &block : cfg->blocks) { + array_free(&block.succs); + ptr_set_destroy(&block.in_params); + ptr_set_destroy(&block.out_params); + } + array_free(&cfg->blocks); + array_free(&cfg->insts); + map_destroy(&cfg->label_block); +} + +gb_internal void check_asm_cfg_build(Ast *at_node, AsmMnemonicAccumulator *acc, AsmCfg *cfg) { + ast_node(at, AsmTemplate, at_node); + + cfg->insts.allocator = heap_allocator(); + cfg->blocks.allocator = heap_allocator(); + map_init(&cfg->label_block); + + bool need_leader = true; + + + // Build basic blocks over the template body. A leader is: the first instruction, any + // instruction preceded by a label, and any instruction following a control transfer. + for (Ast *node : at->instructions) { + if (node->kind == Ast_AsmLabelDecl) { + // Every label between two instructions names the block the *next* instruction + // opens; consecutive labels share it. A trailing label maps to blocks.count. + Entity *le = node->AsmLabelDecl.name->Ident.entity; + if (le != nullptr) { + map_set(&cfg->label_block, le, cast(i32)cfg->blocks.count); + } + need_leader = true; + continue; + } + if (node->kind != Ast_AsmInstruction) { + continue; // directives are straight-line filler; no CFG effect + } + + AstAsmInstruction *instr = &node->AsmInstruction; + AsmInstructionFacts *facts = map_get(&acc->instruction_facts, instr); + // Prefixes and pseudo-macro ops (li/la) carry no facts and never branch. + + if (need_leader || cfg->blocks.count == 0) { + AsmBlock b = {}; + b.first = cast(i32)cfg->insts.count; + b.last = cast(i32)cfg->insts.count; + b.succs.allocator = heap_allocator(); + array_add(&cfg->blocks, b); + need_leader = false; + } + + i32 bi = cast(i32)cfg->blocks.count - 1; + i32 ii = cast(i32)cfg->insts.count; + array_add(&cfg->insts, instr); + cfg->blocks[bi].last = ii; + + if (facts != nullptr) { + facts->block_id = bi; + if (facts->is_control) { + need_leader = true; // the fall-through after a branch starts a new block + } + } + } + + for_array(bi, cfg->blocks) { // Calculate the edges for the blocks + AsmBlock *b = &cfg->blocks[bi]; + + AstAsmInstruction *last = cfg->insts[b->last]; + AsmInstructionFacts *lf = map_get(&acc->instruction_facts, last); + + i32 branch_succ = -1; + bool fallthrough = true; + + if (lf != nullptr && lf->is_control) { + if (lf->branch_target != nullptr) { + i32 *t = map_get(&cfg->label_block, lf->branch_target); + if (t != nullptr && *t < cast(i32)cfg->blocks.count) { + branch_succ = *t; // in-range internal target ('jmp .l' / 'jz .l') + } + // For `t == blocks.count`, this implies a jump to the implicit end, and is handled as "leaves" below + } + // e.g. jmp/ret/hlt (and, conservatively, call) do not fall through in this model. + if (lf->is_terminal) { + fallthrough = false; + } + } + + if (branch_succ >= 0) { + array_add(&b->succs, branch_succ); + } + if (fallthrough) { + i32 next = cast(i32)bi + 1; + if (next < cast(i32)cfg->blocks.count) { + array_add(&b->succs, next); + } + } + } + + if (cfg->blocks.count != 0) { // Reachability determination + Array stack = {}; + stack.allocator = heap_allocator(); + defer (array_free(&stack)); + + cfg->blocks[0].reachable = true; + array_add(&stack, cast(i32)0); + while (stack.count > 0) { + i32 bi = stack[stack.count-1]; + stack.count -= 1; + for (i32 s : cfg->blocks[bi].succs) { + if (s >= 0 && s < cast(i32)cfg->blocks.count && !cfg->blocks[s].reachable) { + cfg->blocks[s].reachable = true; + array_add(&stack, s); + } + } + } + } +} + +gb_internal bool check_asm_cfg_block_leaves(AsmCfg *cfg, AsmMnemonicAccumulator *acc, i32 bi) { + AsmBlock const *b = &cfg->blocks[bi]; + AstAsmInstruction *last = cfg->insts[b->last]; + AsmInstructionFacts *lf = map_get(&acc->instruction_facts, last); + + if (lf != nullptr && lf->branch_target != nullptr) { + i32 *t = map_get(&cfg->label_block, lf->branch_target); + if (t != nullptr && *t >= cast(i32)cfg->blocks.count) { + return true; // 'jmp .end' — falls into the implicit return + } + } + bool terminal = (lf != nullptr) && lf->is_terminal; + if (!terminal && bi > cast(i32)cfg->blocks.count) { + return true; // straight-line / conditional tail with nothing after it + } + return false; +} + +template +gb_internal void check_asm_cfg_report_undef_reg(AsmCtx *asm_ctx, Entity *tmpl_entity, + AstAsmInstruction *instr, String name, u16 bit) { + char const *rname = asm_ctx->clobber_reg_bit_name(bit); + String owner = {}; + char const *role = nullptr; + for (auto const &ed : tmpl_entity->AsmTemplate.decls) { + if (ed.pin.len == 0 || ed.entity == nullptr) { + continue; + } + if (asm_ctx->clobber_bit_for_reg_name(ed.pin) != bit) { + continue; + } + if (ed.param_group == AsmTemplateEntityDeclParamGroup_Output && ed.tie < 0) { + owner = ed.entity->token.string; + role = "output"; + break; + } + if (ed.param_group == AsmTemplateEntityDeclParamGroup_Scratch && ed.view_of < 0) { + owner = ed.entity->token.string; + role = "scratch"; + break; + } + } + if (role != nullptr) { + error(instr->name, + "'%.*s' implicitly reads %%%s, which is bound to the %s parameter '%.*s', " + "but nothing writes %%%s on all paths reaching here; write to it (e.g. into '%.*s') first", + LIT(name), rname, role, LIT(owner), rname, LIT(owner)); + } else { + error(instr->name, + "'%.*s' implicitly reads %%%s, but nothing in this template produces a value for it " + "on all paths reaching here; pin an input to %%%s, or write %%%s first", + LIT(name), rname, rname, rname); + } +} + +template +gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *entity, AsmCfg *cfg, + AsmMnemonicAccumulator *acc) { + GB_ASSERT(entity->kind == Entity_AsmTemplate); + auto const &decls = entity->AsmTemplate.decls; + bool diverging = entity->type->Proc.diverging; + + if (cfg->blocks.count == 0) { + // With an empty body, the CFG cannot really do nothing + if (diverging && !acc->saw_any_instructions) { + error(entity->token, "This asm template is declared as diverging (-> !) but its body is empty and cannot diverge"); + } + return; + } + + if (decls.count > 64) { + error(entity->token, "'asm' templates cannot have more than 64 total parameter declarations, got %td", decls.count); + return; + } + u16 const REG_TOP = asm_ctx->CLOBBER_REGS_NAMED; + + PtrMap entity_to_index = {}; + map_init(&entity_to_index); + defer (map_destroy(&entity_to_index)); + u64 universe_pm = 0; + for_array(i, decls) { + if (decls[i].entity != nullptr) { + map_set(&entity_to_index, decls[i].entity, cast(i32)i); + universe_pm |= (cast(u64)1 << i); + } + } + auto bit_of = [&](Entity *e) -> u64 { + i32 *ix = map_get(&entity_to_index, e); + return ix ? (cast(u64)1 << *ix) : cast(u64)0; + }; + + // NOTE(bill): entry seed intiailization which mirrors the linear seeding of defined_regs + u16 seed_regs = 0; + u64 seed_pm = 0; + for (auto const &ed : decls) { + if (ed.no_init) { + seed_pm |= bit_of(ed.entity); + if (ed.pin.len != 0) { + seed_regs |= asm_ctx->clobber_bit_for_reg_name(ed.pin); + } + } + switch (ed.param_group) { + case AsmTemplateEntityDeclParamGroup_Input: + seed_pm |= bit_of(ed.entity); + if (ed.pin.len != 0) { + seed_regs |= asm_ctx->clobber_bit_for_reg_name(ed.pin); + } + break; + case AsmTemplateEntityDeclParamGroup_Output: + // NOTE(bill): input provides the value + if (ed.tie >= 0) { + seed_pm |= bit_of(ed.entity); + } + break; + } + } + + isize const n = cfg->blocks.count; + + auto in_regs = slice_make(heap_allocator(), n); defer (slice_free(&in_regs, heap_allocator())); + auto out_regs = slice_make(heap_allocator(), n); defer (slice_free(&out_regs, heap_allocator())); + auto gen_regs = slice_make(heap_allocator(), n); defer (slice_free(&gen_regs, heap_allocator())); + auto in_pm = slice_make(heap_allocator(), n); defer (slice_free(&in_pm, heap_allocator())); + auto out_pm = slice_make(heap_allocator(), n); defer (slice_free(&out_pm, heap_allocator())); + auto gen_pm = slice_make(heap_allocator(), n); defer (slice_free(&gen_pm, heap_allocator())); + + // predecessors, restricted to reachable blocks + auto preds = slice_make>(heap_allocator(), n); + for_array(i, preds) { + preds[i].allocator = heap_allocator(); + } + defer ({ + for_array(i, preds) { + array_free(&preds[i]); + } + slice_free(&preds, heap_allocator()); + }); + for_array(bi, cfg->blocks) { + AsmBlock *block = &cfg->blocks[bi]; + if (!block->reachable) { + continue; + } + for (i32 s : block->succs) { + if (0 <= s && s < cast(i32)n && + cfg->blocks[s].reachable) { + array_add(&preds[s], cast(i32)bi); + } + } + } + + for_array(bi, cfg->blocks) { + u16 gr = 0; + u64 gp = 0; + AsmBlock const &b = cfg->blocks[bi]; + for (i32 ii = b.first; ii <= b.last; ii++) { + AsmInstructionFacts *f = map_get(&acc->instruction_facts, cfg->insts[ii]); + if (f == nullptr) { + continue; + } + gr |= f->gen_regs; + for (Entity *pe : f->gen_params) { + gp |= bit_of(pe); + } + } + gen_regs[bi] = gr; + gen_pm[bi] = gp; + } + + // NOTE(bill): initialize the blocks + // entry is from the seeds and every other reachable block from TOP (intersection) + for_array(bi, cfg->blocks) { + if (!cfg->blocks[bi].reachable) { + continue; + } + if (bi == 0) { + in_regs[bi] = seed_regs; + in_pm[bi] = seed_pm; + } else { + in_regs[bi] = REG_TOP; + in_pm[bi] = universe_pm; + } + out_regs[bi] = in_regs[bi] | gen_regs[bi]; + out_pm[bi] = in_pm[bi] | gen_pm[bi]; + } + + // forward must-analysis: in = AND(preds.out); out = in | gen. Iterate to fixpoint. + bool changed = true; + while (changed) { + changed = false; + for_array(bi, cfg->blocks) { + if (!cfg->blocks[bi].reachable) { + continue; + } + + u16 nin_r = seed_regs; + u64 nin_p = seed_pm; + if (bi != 0) { + nin_r = REG_TOP; + nin_p = universe_pm; + for (i32 p : preds[bi]) { + nin_r &= out_regs[p]; + nin_p &= out_pm[p]; + } + } + u16 nout_r = nin_r | gen_regs[bi]; + u64 nout_p = nin_p | gen_pm[bi]; + + if (nin_r != in_regs[bi] || + nin_p != in_pm[bi] || + nout_r != out_regs[bi] || + nout_p != out_pm[bi]) { + in_regs[bi] = nin_r; + in_pm[bi] = nin_p; + out_regs[bi] = nout_r; + out_pm[bi] = nout_p; + changed = true; + } + } + } + + // NOTE(bill): publish the register masks and materialise the parameter sets onto the blocks + for_array(bi, cfg->blocks) { + AsmBlock *b = &cfg->blocks[bi]; + b->in_defs = in_regs[bi]; + b->out_defs = out_regs[bi]; + if (!b->reachable) { + continue; + } + for_array(i, decls) { + Entity *e = decls[i].entity; + if (e == nullptr) { + continue; + } + if (((in_pm[bi] >> i) & 1) != 0) { + ptr_set_add(&b->in_params, e); + } + if (((out_pm[bi] >> i) & 1) != 0) { + ptr_set_add(&b->out_params, e); + } + } + } + + // NOTE(bill): unreachable code + for (AsmBlock &block : cfg->blocks) { + if (block.reachable) { + continue; + } + AstAsmInstruction *first = cfg->insts[block.first]; + if (block.first == block.last) { + warning(first->name, "The asm instruction is unreachable within this block"); + } else { + warning(first->name, "The asm instructions are unreachable within this block"); + } + } + + { // NOTE(bill): read-before-write, definite-assignment across the whole CFG + PtrSet reported_params = {}; + defer (ptr_set_destroy(&reported_params)); + + u16 reported_regs = 0; + + for_array(bi, cfg->blocks) { + AsmBlock const &b = cfg->blocks[bi]; + if (!b.reachable) { + continue; + } + + u16 run_regs = in_regs[bi]; + u64 run_pm = in_pm[bi]; + + for (i32 ii = b.first; ii <= b.last; ii++) { + AstAsmInstruction *instr = cfg->insts[ii]; + AsmInstructionFacts *f = map_get(&acc->instruction_facts, instr); + if (f == nullptr) { + continue; + } + + u16 undef = f->read_regs & REG_TOP & ~run_regs & ~reported_regs; + for (u16 bit = 1; bit != 0; bit <<= 1) { + if ((undef & bit) == 0) { + continue; + } + check_asm_cfg_report_undef_reg(asm_ctx, entity, instr, f->name, bit); + reported_regs |= bit; + } + + for (Entity *pe : f->read_params) { + i32 *ix = map_get(&entity_to_index, pe); + if (ix == nullptr) { + continue; + } + if (((run_pm >> *ix) & 1) == 0 && !ptr_set_exists(&reported_params, pe)) { + Ast *loc = instr->name; + for (Ast *op : instr->operands) { + if (entity_of_node(op) == pe) { loc = op; break; } + } + error(loc, "'%.*s' reads '%.*s' before it is assigned; its initial value is undefined", LIT(f->name), LIT(pe->token.string)); + ptr_set_add(&reported_params, pe); + } + } + + run_regs |= f->gen_regs; + for (Entity *pe : f->gen_params) { + run_pm |= bit_of(pe); + } + } + } + } + + { // NOTE(bill): Collect the template's return points reachable blocks that leave via the end + u16 exit_regs = REG_TOP; + u64 exit_pm = universe_pm; + bool any_exit = false; + for_array(bi, cfg->blocks) { + if (!cfg->blocks[bi].reachable) { + continue; + } + if (!check_asm_cfg_block_leaves(cfg, acc, cast(i32)bi)) { + continue; + } + any_exit = true; + exit_regs &= out_regs[bi]; + exit_pm &= out_pm[bi]; + } + + // NOTE(bill): Outputs must be assigned on every path that returns + if (any_exit && !diverging) { + for (auto const &ed : decls) { + if (ed.param_group != AsmTemplateEntityDeclParamGroup_Output) { + continue; + } + if (ed.tie >= 0 || ed.no_init) { + continue; + } + + bool written; + if (ed.pin.len != 0) { + u16 bit = asm_ctx->clobber_bit_for_reg_name(ed.pin); + written = (bit != 0) && (exit_regs & bit) != 0; + } else { + written = (exit_pm & bit_of(ed.entity)) != 0; + } + if (!written) { + error(ed.entity->token, + "'asm' output parameter '%.*s' is not assigned on all paths through this template; " + "its value is undefined", + LIT(ed.entity->token.string)); + } + } + } + } + + if (diverging) { // No reachable path may return / fall off the end + bool any_leak = false; + for_array(bi, cfg->blocks) { + if (cfg->blocks[bi].reachable && check_asm_cfg_block_leaves(cfg, acc, cast(i32)bi)) { + any_leak = true; + break; + } + } + if (any_leak) { + error(entity->token, + "This asm template is declared diverging (-> !) but a reachable path can fall through the end; " + "end every path with an unconditional jump, return, or halt"); + } + } +} \ No newline at end of file diff --git a/src/entity.cpp b/src/entity.cpp index b70f09045..95b5bf6b7 100644 --- a/src/entity.cpp +++ b/src/entity.cpp @@ -342,6 +342,8 @@ struct Entity { String name; Ast *node; Ast *parent; + + i32 asm_block_index; } Label; struct { Ast *node; @@ -577,6 +579,7 @@ gb_internal Entity *alloc_entity_label(Scope *scope, Token token, Type *type, As Entity *entity = alloc_entity(Entity_Label, scope, token, type); entity->Label.node = node; entity->Label.parent = parent; + entity->Label.asm_block_index = -1; entity->state = EntityState_Resolved; return entity; } From 7fcf65d2c2cc65dcfb0de9b2b075a513cb698c51 Mon Sep 17 00:00:00 2001 From: gingerBill Date: Mon, 24 Aug 2026 22:13:57 +0100 Subject: [PATCH 13/33] asm: General clean up of the CFG code and remove redundant calculations --- src/check_asm.cpp | 114 ++++++++++++---------------------- src/check_asm_cfg.cpp | 141 +++++++++++++++++++++++++++--------------- 2 files changed, 132 insertions(+), 123 deletions(-) diff --git a/src/check_asm.cpp b/src/check_asm.cpp index e57839569..2b0fde8d7 100644 --- a/src/check_asm.cpp +++ b/src/check_asm.cpp @@ -1110,7 +1110,7 @@ template gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tmpl_entity, AstAsmInstruction *instr, u16 mnemonic, u16 pseudo_mnemonic, Slice const &operands, u8 previous_prefix, Ast *previous_prefix_instr, - AsmMnemonicAccumulator *asm_acc) { + AsmCfg *cfg) { GB_ASSERT(mnemonic > 0); auto forms = asm_ctx->encoding_forms(mnemonic); auto clobber_forms = asm_ctx->clobber_forms(mnemonic); @@ -1140,7 +1140,7 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm break; } - map_set(&asm_acc->instruction_facts, instr, facts); + map_set(&cfg->instruction_facts, instr, facts); }); @@ -1578,14 +1578,12 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm // through a parameter pointer does NOT require stack realignment, so // implies_clobber_memory() is intentionally NOT used here. if (clobber.is_call_or_mem()) { - asm_acc->saw_call_or_mem = true; + cfg->saw_call_or_mem = true; } u16 pinned_mask = 0; - for (auto const &ed : tmpl_entity->AsmTemplate.decls) { - if (ed.pin.len != 0) { - pinned_mask |= asm_ctx->clobber_bit_for_reg_name(ed.pin); - } + for_array(i, tmpl_entity->AsmTemplate.decls) { + pinned_mask |= asm_decl_resolve_pin_bit(asm_ctx, tmpl_entity->AsmTemplate.decls, cast(i32)i); } u16 produced = cast(u16)clobber.implicit_wr & asm_ctx->CLOBBER_REGS_NAMED; @@ -1593,50 +1591,25 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm u16 written_ops = cast(u16)clobber.written; u16 pinned_param_writes = 0; + auto const &decls = tmpl_entity->AsmTemplate.decls; for_array(i, operands) { int tslot = user_operand_target_index(cast(int)i); if (tslot < 0 || tslot >= 4 || (written_ops & (1u << tslot)) == 0) { continue; } - auto const &op = operands[i]; - - Ast *e = op.expr; + Ast *e = operands[i].expr; if (e != nullptr && e->kind == Ast_AsmRegister) { u16 b = asm_ctx->clobber_bit_for_reg_name(e->AsmRegister.name.string); produced |= b; explicit_writes |= b; continue; } - - // NOTE(bill): A write through a pinned parameter (or a width-view of one) - // defines that parameter's physical register for the read-before-write check only - auto written_pinned_reg_bit = [&](Operand const &op) -> u16 { - Entity *pe = entity_of_node(op.expr); - if (pe == nullptr || pe->kind != Entity_Variable) { - return 0; - } - auto const &decls = tmpl_entity->AsmTemplate.decls; - for_array(di, decls) { - auto const &ed = decls[di]; - if (ed.entity != pe) { - continue; - } - if (ed.pin.len != 0) { - return asm_ctx->clobber_bit_for_reg_name(ed.pin); - } - // NOTE(bill): A width-view carries no pin of its own and thus it aliases its source's register. - if (ed.view_of >= 0 && ed.view_of < cast(i32)decls.count) { - String src_pin = decls[ed.view_of].pin; - if (src_pin.len != 0) { - return asm_ctx->clobber_bit_for_reg_name(src_pin); - } - } - return 0; - } - return 0; - }; - - pinned_param_writes |= written_pinned_reg_bit(operands[i]); + Entity *pe = entity_of_node(operands[i].expr); + if (pe != nullptr && pe->kind == Entity_Variable) { + i32 di = -1; + check_asm_find_group(pe, decls, &di); // reuse existing index finder + pinned_param_writes |= asm_decl_resolve_pin_bit(asm_ctx, decls, di); + } } if (is_pseudo && @@ -1704,15 +1677,15 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm // redundant-#clobber hint. Union across the template; pinned regs excluded // so a legitimate output pin is never called "redundant". u16 implicit_wr = cast(u16)clobber.implicit_wr & asm_ctx->CLOBBER_REGS_NAMED; - asm_acc->implicit_clobbered_regs |= implicit_wr & ~pinned_mask; + cfg->implicit_clobbered_regs |= implicit_wr & ~pinned_mask; // Approximate staleness. An output that was explicitly produced (literal %reg write) // and is later implicitly clobbered — without this same instruction re-producing it — // is marked stale. Explicit re-production clears it. Implicitly-produced outputs // (RDTSC->RDX) are never tracked, so they never false-fire. - asm_acc->explicitly_produced_regs |= explicit_writes; - asm_acc->stale_outputs &= ~explicit_writes; - asm_acc->stale_outputs |= implicit_wr & asm_acc->explicitly_produced_regs & ~explicit_writes; + cfg->explicitly_produced_regs |= explicit_writes; + cfg->stale_outputs &= ~explicit_writes; + cfg->stale_outputs |= implicit_wr & cfg->explicitly_produced_regs & ~explicit_writes; } { @@ -1734,7 +1707,7 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm asm_ctx->clobber_implicit_regs(&tmpl_entity->AsmTemplate.clobber_registers_set, produced); // Purity inference - if (asm_acc->can_be_pure) { + if (cfg->can_be_pure) { // NOTE(bill): Only the first violating instruction is recorded // The later ones don't overwrite the reason. char const *why = nullptr; @@ -1760,9 +1733,9 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm } if (why != nullptr) { - asm_acc->can_be_pure = false; - asm_acc->impure_reason = why; - asm_acc->impure_reason_node = instr->name; + cfg->can_be_pure = false; + cfg->impure_reason = why; + cfg->impure_reason_node = instr->name; } } return; @@ -2416,11 +2389,16 @@ gb_internal void check_asm_template(AsmCtx *asm_ctx, CheckerContext *ctx, Entity } } - AsmMnemonicAccumulator asm_acc = {}; - map_init(&asm_acc.instruction_facts); - defer (map_destroy(&asm_acc.instruction_facts)); + // NOTE(bill, 2026-08-24): Construct a control-flow graph (CFG) from the instructions + // to do further analysis which is not possible with an conservative straight-line approximation + // Using a CFG is a much sounder approach for calculating: + // * reads before writes + // * divergence + // * unreachable code - asm_acc.can_be_pure = true; + AsmCfg cfg = {}; + asm_cfg_init(&cfg); + defer (asm_cfg_destroy(&cfg)); // collect label decls for (Ast *instruction_ : at->instructions) { @@ -2484,9 +2462,9 @@ gb_internal void check_asm_template(AsmCtx *asm_ctx, CheckerContext *ctx, Entity instr->suffix_flags = suffix_flags; check_mnemonic(asm_ctx, ctx, entity, instr, mnemonic, 0, slice_from_array(operands), previous_prefix, previous_prefix_instr, - &asm_acc); + &cfg); - asm_acc.saw_any_instructions = true; + cfg.saw_any_instructions = true; previous_prefix = 0; previous_prefix_instr = nullptr; @@ -2498,9 +2476,9 @@ gb_internal void check_asm_template(AsmCtx *asm_ctx, CheckerContext *ctx, Entity u16 target_mnemonic = cast(u16)alias.target; check_mnemonic(asm_ctx, ctx, entity, instr, target_mnemonic, pseudo_mnemonic, slice_from_array(operands), previous_prefix, previous_prefix_instr, - &asm_acc); + &cfg); - asm_acc.saw_any_instructions = true; + cfg.saw_any_instructions = true; previous_prefix = 0; previous_prefix_instr = nullptr; @@ -2508,7 +2486,7 @@ gb_internal void check_asm_template(AsmCtx *asm_ctx, CheckerContext *ctx, Entity instr->suffix_flags = suffix_flags; check_pseudo_macro_mnemonic(asm_ctx, entity, instr, slice_from_array(operands)); - asm_acc.saw_any_instructions = true; + cfg.saw_any_instructions = true; previous_prefix = 0; previous_prefix_instr = nullptr; @@ -2618,18 +2596,8 @@ gb_internal void check_asm_template(AsmCtx *asm_ctx, CheckerContext *ctx, Entity error(previous_prefix_instr, "A prefix must be immediately followed by an instruction, but the template ended"); } - // NOTE(bill, 2026-08-24): Construct a control-flow graph (CFG) from the instructions - // to do further analysis which is not possible with an conservative straight-line approximation - // Using a CFG is a much sounder approach for calculating: - // * reads before writes - // * divergence - // * unreachable code - - AsmCfg cfg = {}; - defer (asm_cfg_destroy(&cfg)); - check_asm_cfg_build(d->init_expr, &asm_acc, &cfg); - check_asm_cfg_analyse(asm_ctx, ctx, entity, &cfg, &asm_acc); - + check_asm_cfg_build(asm_ctx, &cfg, d->init_expr, entity); + check_asm_cfg_analyse(asm_ctx, &cfg, ctx, entity); bool vet_unused = false; { @@ -2710,7 +2678,7 @@ gb_internal void check_asm_template(AsmCtx *asm_ctx, CheckerContext *ctx, Entity "Please add #volatile if the effect is intended."); } - if (entity->AsmTemplate.is_align_stack && !asm_acc.saw_call_or_mem) { + if (entity->AsmTemplate.is_align_stack && !cfg.saw_call_or_mem) { warning(entity->token, "#align_stack is redundant; this template makes no call and touches no memory " "that would require the stack to be realigned"); @@ -2720,12 +2688,12 @@ gb_internal void check_asm_template(AsmCtx *asm_ctx, CheckerContext *ctx, Entity bool declared_effects = entity->AsmTemplate.is_volatile || entity->AsmTemplate.clobber_memory || entity->AsmTemplate.has_observable_side_effect; - bool is_pure = asm_acc.can_be_pure && !declared_effects && !type->Proc.diverging; + bool is_pure = cfg.can_be_pure && !declared_effects && !type->Proc.diverging; entity->AsmTemplate.is_pure = is_pure; if (is_pure_annotated && !is_pure) { - Ast *node = asm_acc.impure_reason_node; - char const *why = asm_acc.impure_reason; + Ast *node = cfg.impure_reason_node; + char const *why = cfg.impure_reason; if (why == nullptr) { if (type->Proc.diverging) { why = "it is declared diverging (-> !) and computes no outputs"; diff --git a/src/check_asm_cfg.cpp b/src/check_asm_cfg.cpp index a02994ca6..8d4e4eb39 100644 --- a/src/check_asm_cfg.cpp +++ b/src/check_asm_cfg.cpp @@ -26,8 +26,7 @@ struct AsmInstructionFacts { i32 block_id; }; - -struct AsmMnemonicAccumulator { +struct AsmCfg { // Union of registers implicitly clobbered by matched forms (for redundant-#clobber hints). u16 implicit_clobbered_regs; u16 explicitly_produced_regs; @@ -46,14 +45,24 @@ struct AsmMnemonicAccumulator { Ast * impure_reason_node; PtrMap instruction_facts; -}; -struct AsmCfg { Array insts; // program-order (only for fact-carrying instrs) Array blocks; PtrMap label_block; // key: Entity_Label* + + PtrMap entity_to_index; + Array decl_pin_bit; + u64 universe_pm; }; +gb_internal void asm_cfg_init(AsmCfg *cfg) { + map_init(&cfg->instruction_facts); + map_init(&cfg->entity_to_index); + cfg->decl_pin_bit.allocator = heap_allocator(); + cfg->can_be_pure = true; +}; + + gb_internal void asm_cfg_destroy(AsmCfg *cfg) { for (auto &block : cfg->blocks) { array_free(&block.succs); @@ -63,18 +72,62 @@ gb_internal void asm_cfg_destroy(AsmCfg *cfg) { array_free(&cfg->blocks); array_free(&cfg->insts); map_destroy(&cfg->label_block); + map_destroy(&cfg->instruction_facts); + map_destroy(&cfg->entity_to_index); + array_free(&cfg->decl_pin_bit); } -gb_internal void check_asm_cfg_build(Ast *at_node, AsmMnemonicAccumulator *acc, AsmCfg *cfg) { +// The physical-register bit a decl is pinned to. A width-view carries no pin of +// its own; it inherits its source decl's pin. Returns 0 for unpinned decls. +template +gb_internal u16 asm_decl_resolve_pin_bit(AsmCtx *asm_ctx, Array const &decls, i32 di) { + if (di < 0 || di >= cast(i32)decls.count) { + return 0; + } + auto const &ed = decls[di]; + if (ed.pin.len != 0) { + return asm_ctx->clobber_bit_for_reg_name(ed.pin); + } + if (ed.view_of >= 0 && ed.view_of < cast(i32)decls.count) { + String src_pin = decls[ed.view_of].pin; + if (src_pin.len != 0) { + return asm_ctx->clobber_bit_for_reg_name(src_pin); + } + } + return 0; +} + +template +gb_internal void asm_cfg_populate_decls(AsmCtx *asm_ctx, AsmCfg *cfg, Entity *entity) { + auto const &decls = entity->AsmTemplate.decls; + cfg->universe_pm = 0; + if (decls.count > 64) { + // NOTE(bill): check_asm_cfg_analyse will err on this since this is exceed the maximum number of declarations + return; + } + array_resize(&cfg->decl_pin_bit, decls.count); + for_array(i, decls) { + Entity *e = decls[i].entity; + cfg->decl_pin_bit[i] = asm_decl_resolve_pin_bit(asm_ctx, decls, cast(i32)i); + if (e != nullptr) { + map_set(&cfg->entity_to_index, e, cast(i32)i); + cfg->universe_pm |= (cast(u64)1 << i); + } + } +} + +template +gb_internal void check_asm_cfg_build(AsmCtx *asm_ctx, AsmCfg *cfg, Ast *at_node, Entity *entity) { ast_node(at, AsmTemplate, at_node); + asm_cfg_populate_decls(asm_ctx, cfg, entity); + cfg->insts.allocator = heap_allocator(); cfg->blocks.allocator = heap_allocator(); map_init(&cfg->label_block); bool need_leader = true; - // Build basic blocks over the template body. A leader is: the first instruction, any // instruction preceded by a label, and any instruction following a control transfer. for (Ast *node : at->instructions) { @@ -93,7 +146,7 @@ gb_internal void check_asm_cfg_build(Ast *at_node, AsmMnemonicAccumulator *acc, } AstAsmInstruction *instr = &node->AsmInstruction; - AsmInstructionFacts *facts = map_get(&acc->instruction_facts, instr); + AsmInstructionFacts *facts = map_get(&cfg->instruction_facts, instr); // Prefixes and pseudo-macro ops (li/la) carry no facts and never branch. if (need_leader || cfg->blocks.count == 0) { @@ -122,7 +175,7 @@ gb_internal void check_asm_cfg_build(Ast *at_node, AsmMnemonicAccumulator *acc, AsmBlock *b = &cfg->blocks[bi]; AstAsmInstruction *last = cfg->insts[b->last]; - AsmInstructionFacts *lf = map_get(&acc->instruction_facts, last); + AsmInstructionFacts *lf = map_get(&cfg->instruction_facts, last); i32 branch_succ = -1; bool fallthrough = true; @@ -172,10 +225,10 @@ gb_internal void check_asm_cfg_build(Ast *at_node, AsmMnemonicAccumulator *acc, } } -gb_internal bool check_asm_cfg_block_leaves(AsmCfg *cfg, AsmMnemonicAccumulator *acc, i32 bi) { +gb_internal bool check_asm_cfg_block_leaves(AsmCfg *cfg, i32 bi) { AsmBlock const *b = &cfg->blocks[bi]; AstAsmInstruction *last = cfg->insts[b->last]; - AsmInstructionFacts *lf = map_get(&acc->instruction_facts, last); + AsmInstructionFacts *lf = map_get(&cfg->instruction_facts, last); if (lf != nullptr && lf->branch_target != nullptr) { i32 *t = map_get(&cfg->label_block, lf->branch_target); @@ -184,23 +237,23 @@ gb_internal bool check_asm_cfg_block_leaves(AsmCfg *cfg, AsmMnemonicAccumulator } } bool terminal = (lf != nullptr) && lf->is_terminal; - if (!terminal && bi > cast(i32)cfg->blocks.count) { + if (!terminal && (bi+1 >= cast(i32)cfg->blocks.count)) { return true; // straight-line / conditional tail with nothing after it } return false; } template -gb_internal void check_asm_cfg_report_undef_reg(AsmCtx *asm_ctx, Entity *tmpl_entity, +gb_internal void check_asm_cfg_report_undef_reg(AsmCtx *asm_ctx, AsmCfg *cfg, Entity *tmpl_entity, AstAsmInstruction *instr, String name, u16 bit) { char const *rname = asm_ctx->clobber_reg_bit_name(bit); String owner = {}; char const *role = nullptr; - for (auto const &ed : tmpl_entity->AsmTemplate.decls) { - if (ed.pin.len == 0 || ed.entity == nullptr) { - continue; - } - if (asm_ctx->clobber_bit_for_reg_name(ed.pin) != bit) { + + auto const &decls = tmpl_entity->AsmTemplate.decls; + for_array(i, decls) { + auto const &ed = decls[i]; + if (ed.entity == nullptr || cfg->decl_pin_bit[i] != bit) { continue; } if (ed.param_group == AsmTemplateEntityDeclParamGroup_Output && ed.tie < 0) { @@ -228,15 +281,14 @@ gb_internal void check_asm_cfg_report_undef_reg(AsmCtx *asm_ctx, Entity *tmpl_en } template -gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *entity, AsmCfg *cfg, - AsmMnemonicAccumulator *acc) { +gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, AsmCfg *cfg, CheckerContext *ctx, Entity *entity) { GB_ASSERT(entity->kind == Entity_AsmTemplate); auto const &decls = entity->AsmTemplate.decls; bool diverging = entity->type->Proc.diverging; if (cfg->blocks.count == 0) { // With an empty body, the CFG cannot really do nothing - if (diverging && !acc->saw_any_instructions) { + if (diverging && !cfg->saw_any_instructions) { error(entity->token, "This asm template is declared as diverging (-> !) but its body is empty and cannot diverge"); } return; @@ -248,40 +300,28 @@ gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, CheckerContext *ctx, Ent } u16 const REG_TOP = asm_ctx->CLOBBER_REGS_NAMED; - PtrMap entity_to_index = {}; - map_init(&entity_to_index); - defer (map_destroy(&entity_to_index)); - u64 universe_pm = 0; - for_array(i, decls) { - if (decls[i].entity != nullptr) { - map_set(&entity_to_index, decls[i].entity, cast(i32)i); - universe_pm |= (cast(u64)1 << i); - } - } + u64 const universe_pm = cfg->universe_pm; auto bit_of = [&](Entity *e) -> u64 { - i32 *ix = map_get(&entity_to_index, e); + i32 *ix = map_get(&cfg->entity_to_index, e); return ix ? (cast(u64)1 << *ix) : cast(u64)0; }; // NOTE(bill): entry seed intiailization which mirrors the linear seeding of defined_regs u16 seed_regs = 0; u64 seed_pm = 0; - for (auto const &ed : decls) { + for_array(i, decls) { + auto const &ed = decls[i]; + u16 pin_bit = cfg->decl_pin_bit[i]; if (ed.no_init) { seed_pm |= bit_of(ed.entity); - if (ed.pin.len != 0) { - seed_regs |= asm_ctx->clobber_bit_for_reg_name(ed.pin); - } + seed_regs |= pin_bit; } switch (ed.param_group) { case AsmTemplateEntityDeclParamGroup_Input: seed_pm |= bit_of(ed.entity); - if (ed.pin.len != 0) { - seed_regs |= asm_ctx->clobber_bit_for_reg_name(ed.pin); - } + seed_regs |= pin_bit; break; case AsmTemplateEntityDeclParamGroup_Output: - // NOTE(bill): input provides the value if (ed.tie >= 0) { seed_pm |= bit_of(ed.entity); } @@ -327,7 +367,7 @@ gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, CheckerContext *ctx, Ent u64 gp = 0; AsmBlock const &b = cfg->blocks[bi]; for (i32 ii = b.first; ii <= b.last; ii++) { - AsmInstructionFacts *f = map_get(&acc->instruction_facts, cfg->insts[ii]); + AsmInstructionFacts *f = map_get(&cfg->instruction_facts, cfg->insts[ii]); if (f == nullptr) { continue; } @@ -444,7 +484,7 @@ gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, CheckerContext *ctx, Ent for (i32 ii = b.first; ii <= b.last; ii++) { AstAsmInstruction *instr = cfg->insts[ii]; - AsmInstructionFacts *f = map_get(&acc->instruction_facts, instr); + AsmInstructionFacts *f = map_get(&cfg->instruction_facts, instr); if (f == nullptr) { continue; } @@ -454,12 +494,12 @@ gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, CheckerContext *ctx, Ent if ((undef & bit) == 0) { continue; } - check_asm_cfg_report_undef_reg(asm_ctx, entity, instr, f->name, bit); + check_asm_cfg_report_undef_reg(asm_ctx, cfg, entity, instr, f->name, bit); reported_regs |= bit; } for (Entity *pe : f->read_params) { - i32 *ix = map_get(&entity_to_index, pe); + i32 *ix = map_get(&cfg->entity_to_index, pe); if (ix == nullptr) { continue; } @@ -489,7 +529,7 @@ gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, CheckerContext *ctx, Ent if (!cfg->blocks[bi].reachable) { continue; } - if (!check_asm_cfg_block_leaves(cfg, acc, cast(i32)bi)) { + if (!check_asm_cfg_block_leaves(cfg, cast(i32)bi)) { continue; } any_exit = true; @@ -499,7 +539,8 @@ gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, CheckerContext *ctx, Ent // NOTE(bill): Outputs must be assigned on every path that returns if (any_exit && !diverging) { - for (auto const &ed : decls) { + for_array(i, decls) { + auto const &ed = decls[i]; if (ed.param_group != AsmTemplateEntityDeclParamGroup_Output) { continue; } @@ -507,10 +548,10 @@ gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, CheckerContext *ctx, Ent continue; } - bool written; - if (ed.pin.len != 0) { - u16 bit = asm_ctx->clobber_bit_for_reg_name(ed.pin); - written = (bit != 0) && (exit_regs & bit) != 0; + bool written = false; + u16 bit = cfg->decl_pin_bit[i]; + if (bit != 0) { + written = (exit_regs & bit) != 0; } else { written = (exit_pm & bit_of(ed.entity)) != 0; } @@ -527,7 +568,7 @@ gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, CheckerContext *ctx, Ent if (diverging) { // No reachable path may return / fall off the end bool any_leak = false; for_array(bi, cfg->blocks) { - if (cfg->blocks[bi].reachable && check_asm_cfg_block_leaves(cfg, acc, cast(i32)bi)) { + if (cfg->blocks[bi].reachable && check_asm_cfg_block_leaves(cfg, cast(i32)bi)) { any_leak = true; break; } From a2d9ca86973c87d01afcb09298ba034d8b3d2d17 Mon Sep 17 00:00:00 2001 From: gingerBill Date: Mon, 24 Aug 2026 22:25:53 +0100 Subject: [PATCH 14/33] Remove unneeded hash maps when you can store the data directly on the nodes --- src/check_asm.cpp | 28 ++++++++++++++-------------- src/check_asm_cfg.cpp | 43 +++++++++++++++++++++++++------------------ src/check_expr.cpp | 4 ++++ src/parser.cpp | 1 + src/parser.hpp | 1 + 5 files changed, 45 insertions(+), 32 deletions(-) diff --git a/src/check_asm.cpp b/src/check_asm.cpp index 2b0fde8d7..1d5da9b02 100644 --- a/src/check_asm.cpp +++ b/src/check_asm.cpp @@ -1121,11 +1121,11 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm name = asm_ctx->pseudo_mnemonic_strings[pseudo_mnemonic]; } - AsmInstructionFacts facts = {}; - facts.node = instr; - facts.name = name; - facts.gen_params.allocator = heap_allocator(); - facts.read_params.allocator = heap_allocator(); + AsmInstructionFacts *facts = gb_alloc_item(permanent_allocator(), AsmInstructionFacts); + facts->node = instr; + facts->name = name; + facts->gen_params.allocator = heap_allocator(); + facts->read_params.allocator = heap_allocator(); defer ({ for (auto const &op : operands) { @@ -1136,11 +1136,11 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm if (e == nullptr || e->kind != Entity_Label) { continue; } - facts.branch_target = e; + facts->branch_target = e; break; } - map_set(&cfg->instruction_facts, instr, facts); + instr->facts = facts; }); @@ -1541,7 +1541,7 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm // Handle clobbering from mnemonic auto clobber = clobber_forms[valid_form_index]; - facts.read_regs = cast(u16)clobber.implicit_rd & asm_ctx->CLOBBER_REGS_NAMED; + facts->read_regs = cast(u16)clobber.implicit_rd & asm_ctx->CLOBBER_REGS_NAMED; // NOTE(bill): reads_mem/writes_mem are per-FORM capability bits. // A form with an r/m slot (e.g. add r/m32, imm32) carries them even @@ -1630,7 +1630,7 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm explicit_writes |= synth; } - facts.gen_regs = produced | pinned_param_writes; + facts->gen_regs = produced | pinned_param_writes; // NOTE(bill): mnemonics such as `xor r, r` / `sub r, r` act as zeroing the destination // independent of its prior value: the read is architecturally dead, so it must not count as a use. @@ -1665,10 +1665,10 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm } if (!self_zeroing && (cast(u16)clobber.read & (1u << slot))) { - array_add(&facts.read_params, pe); + array_add(&facts->read_params, pe); } if (cast(u16)clobber.written & (1u << slot)) { - array_add(&facts.gen_params, pe); + array_add(&facts->gen_params, pe); } } @@ -1700,9 +1700,9 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm // A conditional branch reads a flag and can fall through -> not terminal. bool conditional = clobber.is_conditional(); - facts.is_control = control; - facts.is_conditional = conditional; - facts.is_terminal = halt || (control && !conditional); + facts->is_control = control; + facts->is_conditional = conditional; + facts->is_terminal = halt || (control && !conditional); } asm_ctx->clobber_implicit_regs(&tmpl_entity->AsmTemplate.clobber_registers_set, produced); diff --git a/src/check_asm_cfg.cpp b/src/check_asm_cfg.cpp index 8d4e4eb39..1f37ccb3d 100644 --- a/src/check_asm_cfg.cpp +++ b/src/check_asm_cfg.cpp @@ -44,11 +44,8 @@ struct AsmCfg { char const *impure_reason; Ast * impure_reason_node; - PtrMap instruction_facts; - Array insts; // program-order (only for fact-carrying instrs) Array blocks; - PtrMap label_block; // key: Entity_Label* PtrMap entity_to_index; Array decl_pin_bit; @@ -56,7 +53,6 @@ struct AsmCfg { }; gb_internal void asm_cfg_init(AsmCfg *cfg) { - map_init(&cfg->instruction_facts); map_init(&cfg->entity_to_index); cfg->decl_pin_bit.allocator = heap_allocator(); cfg->can_be_pure = true; @@ -71,12 +67,24 @@ gb_internal void asm_cfg_destroy(AsmCfg *cfg) { } array_free(&cfg->blocks); array_free(&cfg->insts); - map_destroy(&cfg->label_block); - map_destroy(&cfg->instruction_facts); map_destroy(&cfg->entity_to_index); array_free(&cfg->decl_pin_bit); } +gb_internal i32 asm_cfg_label_block_index(Entity *entity) { + if (entity != nullptr && entity->kind == Entity_Label) { + return entity->Label.asm_block_index; + } + return -1; +} +gb_internal bool asm_cfg_label_block_index_set(Entity *entity, i32 index) { + if (entity != nullptr && entity->kind == Entity_Label) { + GB_ASSERT(entity->Label.asm_block_index < 0); + entity->Label.asm_block_index = index; + } + return false; +} + // The physical-register bit a decl is pinned to. A width-view carries no pin of // its own; it inherits its source decl's pin. Returns 0 for unpinned decls. template @@ -124,7 +132,6 @@ gb_internal void check_asm_cfg_build(AsmCtx *asm_ctx, AsmCfg *cfg, Ast *at_node, cfg->insts.allocator = heap_allocator(); cfg->blocks.allocator = heap_allocator(); - map_init(&cfg->label_block); bool need_leader = true; @@ -136,7 +143,7 @@ gb_internal void check_asm_cfg_build(AsmCtx *asm_ctx, AsmCfg *cfg, Ast *at_node, // opens; consecutive labels share it. A trailing label maps to blocks.count. Entity *le = node->AsmLabelDecl.name->Ident.entity; if (le != nullptr) { - map_set(&cfg->label_block, le, cast(i32)cfg->blocks.count); + asm_cfg_label_block_index_set(le, cast(i32)cfg->blocks.count); } need_leader = true; continue; @@ -146,7 +153,7 @@ gb_internal void check_asm_cfg_build(AsmCtx *asm_ctx, AsmCfg *cfg, Ast *at_node, } AstAsmInstruction *instr = &node->AsmInstruction; - AsmInstructionFacts *facts = map_get(&cfg->instruction_facts, instr); + AsmInstructionFacts *facts = instr->facts; // Prefixes and pseudo-macro ops (li/la) carry no facts and never branch. if (need_leader || cfg->blocks.count == 0) { @@ -175,16 +182,16 @@ gb_internal void check_asm_cfg_build(AsmCtx *asm_ctx, AsmCfg *cfg, Ast *at_node, AsmBlock *b = &cfg->blocks[bi]; AstAsmInstruction *last = cfg->insts[b->last]; - AsmInstructionFacts *lf = map_get(&cfg->instruction_facts, last); + AsmInstructionFacts *lf = last->facts; i32 branch_succ = -1; bool fallthrough = true; if (lf != nullptr && lf->is_control) { if (lf->branch_target != nullptr) { - i32 *t = map_get(&cfg->label_block, lf->branch_target); - if (t != nullptr && *t < cast(i32)cfg->blocks.count) { - branch_succ = *t; // in-range internal target ('jmp .l' / 'jz .l') + i32 t = asm_cfg_label_block_index(lf->branch_target); + if (0 <= t && t < cast(i32)cfg->blocks.count) { + branch_succ = t; // in-range internal target ('jmp .l' / 'jz .l') } // For `t == blocks.count`, this implies a jump to the implicit end, and is handled as "leaves" below } @@ -228,11 +235,11 @@ gb_internal void check_asm_cfg_build(AsmCtx *asm_ctx, AsmCfg *cfg, Ast *at_node, gb_internal bool check_asm_cfg_block_leaves(AsmCfg *cfg, i32 bi) { AsmBlock const *b = &cfg->blocks[bi]; AstAsmInstruction *last = cfg->insts[b->last]; - AsmInstructionFacts *lf = map_get(&cfg->instruction_facts, last); + AsmInstructionFacts *lf = last->facts; if (lf != nullptr && lf->branch_target != nullptr) { - i32 *t = map_get(&cfg->label_block, lf->branch_target); - if (t != nullptr && *t >= cast(i32)cfg->blocks.count) { + i32 t = asm_cfg_label_block_index(lf->branch_target); + if (t >= 0 && t >= cast(i32)cfg->blocks.count) { return true; // 'jmp .end' — falls into the implicit return } } @@ -367,7 +374,7 @@ gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, AsmCfg *cfg, CheckerCont u64 gp = 0; AsmBlock const &b = cfg->blocks[bi]; for (i32 ii = b.first; ii <= b.last; ii++) { - AsmInstructionFacts *f = map_get(&cfg->instruction_facts, cfg->insts[ii]); + AsmInstructionFacts *f = cfg->insts[ii]->facts; if (f == nullptr) { continue; } @@ -484,7 +491,7 @@ gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, AsmCfg *cfg, CheckerCont for (i32 ii = b.first; ii <= b.last; ii++) { AstAsmInstruction *instr = cfg->insts[ii]; - AsmInstructionFacts *f = map_get(&cfg->instruction_facts, instr); + AsmInstructionFacts *f = instr->facts; if (f == nullptr) { continue; } diff --git a/src/check_expr.cpp b/src/check_expr.cpp index 091ebdac2..937a0eb07 100644 --- a/src/check_expr.cpp +++ b/src/check_expr.cpp @@ -13894,6 +13894,10 @@ gb_internal gbString write_expr_to_string(gbString str, Ast *node, bool shorthan str = gb_string_appendc(str, " = "); str = write_expr_to_string(str, spec->value, shorthand); } + for (Ast *dir : spec->directives) { + str = gb_string_appendc(str, " "); + str = write_expr_to_string(str, dir, shorthand); + } case_end; case_ast_node(clobber, AsmClobber, node); diff --git a/src/parser.cpp b/src/parser.cpp index f8ccb79a6..43fbea7c9 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -548,6 +548,7 @@ gb_internal Ast *clone_ast(Ast *node, AstFile *f) { case Ast_AsmInstruction: n->AsmInstruction.name = clone_ast(n->AsmInstruction.name, f); n->AsmInstruction.operands = clone_ast_array(n->AsmInstruction.operands, f); + n->AsmInstruction.facts = nullptr; break; case Ast_AsmMemoryOperand: n->AsmMemoryOperand.segment_override = clone_ast(n->AsmMemoryOperand.segment_override, f); diff --git a/src/parser.hpp b/src/parser.hpp index ad82834ed..260cb8a85 100644 --- a/src/parser.hpp +++ b/src/parser.hpp @@ -510,6 +510,7 @@ struct AstSplitArgs { u16 mnemonic; \ u8 suffix_flags; \ i32 valid_form_index; \ + struct AsmInstructionFacts *facts; \ }) \ AST_KIND(AsmMemoryOperand, "asm memory operand", struct { \ Token open; \ From 477fd7467bc9a39b199e69e0c7d30c41a187a69d Mon Sep 17 00:00:00 2001 From: gingerBill Date: Mon, 24 Aug 2026 22:32:48 +0100 Subject: [PATCH 15/33] Ignore flag-pinned output --- src/check_asm_cfg.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/check_asm_cfg.cpp b/src/check_asm_cfg.cpp index 1f37ccb3d..b866e1b56 100644 --- a/src/check_asm_cfg.cpp +++ b/src/check_asm_cfg.cpp @@ -554,6 +554,9 @@ gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, AsmCfg *cfg, CheckerCont if (ed.tie >= 0 || ed.no_init) { continue; } + if (ed.pin_flag.len != 0) { + continue; // flag-pinned output: defined by a flags side effect, not a reg/param write + } bool written = false; u16 bit = cfg->decl_pin_bit[i]; From 032566714c12f0f4d7d9fed400b0ab035a6c5dea Mon Sep 17 00:00:00 2001 From: gingerBill Date: Mon, 24 Aug 2026 22:56:49 +0100 Subject: [PATCH 16/33] asm: correctly handle flags in the CFG --- .../riscv/tablegen/cpp-compiler/cpp-gen.odin | 37 ++++-- .../x86/tablegen/cpp-compiler/cpp-gen.odin | 21 ++++ src/asm_tables_amd64.cpp | 21 ++++ src/asm_tables_riscv.cpp | 37 ++++-- src/check_asm.cpp | 5 +- src/check_asm_cfg.cpp | 109 ++++++++++++------ 6 files changed, 175 insertions(+), 55 deletions(-) diff --git a/core/rexcode/isa/riscv/tablegen/cpp-compiler/cpp-gen.odin b/core/rexcode/isa/riscv/tablegen/cpp-compiler/cpp-gen.odin index f4f70d9be..c19194c3c 100644 --- a/core/rexcode/isa/riscv/tablegen/cpp-compiler/cpp-gen.odin +++ b/core/rexcode/isa/riscv/tablegen/cpp-compiler/cpp-gen.odin @@ -120,12 +120,12 @@ main :: proc() { strings.write_string(&sb, "\n") strings.write_string(&sb, """ - enum ClobberFFlags : u8 { - ClobberFFlag_NV = 1<<0, // invalid operation - ClobberFFlag_DZ = 1<<1, // divide by zero - ClobberFFlag_OF = 1<<2, // overflow - ClobberFFlag_UF = 1<<3, // underflow - ClobberFFlag_NX = 1<<4, // inexact + enum ClobberFlags : u8 { + ClobberFlag_NV = 1<<0, // invalid operation + ClobberFlag_DZ = 1<<1, // divide by zero + ClobberFlag_OF = 1<<2, // overflow + ClobberFlag_UF = 1<<3, // underflow + ClobberFlag_NX = 1<<4, // inexact }; enum ClobberRegs : u8 { @@ -174,9 +174,28 @@ main :: proc() { return \"\"; } + u16 flags_from_name(String const &name) { + static const struct { String name; ClobberFlags flag; } table[] = { + // flags: accrued FP exception flags (fcsr[4:0]) + {str_lit(\"nx\"), ClobberFlag_NX}, // Inexact + {str_lit(\"uf\"), ClobberFlag_UF}, // Underflow + {str_lit(\"of\"), ClobberFlag_OF}, // Overflow + {str_lit(\"dz\"), ClobberFlag_DZ}, // Divide by Zero + {str_lit(\"nv\"), ClobberFlag_NV}, // Invalid Operation + }; + + for (auto const &t : table) { + if (name == t.name) { + return cast(u16)t.flag; + } + } + return 0; + } + + i32 flag_bit_from_name(String const &name, i32 *width_) { static const struct { String name; i32 bit; } table[] = { - // fflags: accrued FP exception flags (fcsr[4:0]) + // flags: accrued FP exception flags (fcsr[4:0]) {str_lit(\"nx\"), 0}, // Inexact {str_lit(\"uf\"), 1}, // Underflow {str_lit(\"of\"), 2}, // Overflow @@ -207,14 +226,14 @@ main :: proc() { OperandSet read; // operand slots whose register/CSR/mem-base is read ClobberRegs implicit_wr; // implicit reg writes (ra on C.JAL/C.JALR) ClobberRegs implicit_rd; // implicit reg reads (sp on the *SP forms) - ClobberFFlags fflags_wr; // accrued exception flags this op may raise + ClobberFlags flags_wr; // accrued exception flags this op may raise bool reads_frm; // consumes the dynamic rounding mode from fcsr bool writes_mem; bool reads_mem; SideEffectFlags side_effects; bool implies_clobber_flags() const { - return (fflags_wr != 0); + return (flags_wr != 0); } bool implies_clobber_memory() const { return writes_mem || reads_mem || diff --git a/core/rexcode/isa/x86/tablegen/cpp-compiler/cpp-gen.odin b/core/rexcode/isa/x86/tablegen/cpp-compiler/cpp-gen.odin index d64c6b2e4..558c8cd7e 100644 --- a/core/rexcode/isa/x86/tablegen/cpp-compiler/cpp-gen.odin +++ b/core/rexcode/isa/x86/tablegen/cpp-compiler/cpp-gen.odin @@ -237,6 +237,27 @@ main :: proc() { return \"\"; } + u16 flag_from_name(String const &name) { + static const struct {String name; ClobberFlags flag; } table[] = { + {str_lit(\"c\"), ClobberFlag_CF}, // Carry + {str_lit(\"p\"), ClobberFlag_PF}, // Parity + {str_lit(\"a\"), ClobberFlag_AF}, // Auxiliary Carry + {str_lit(\"z\"), ClobberFlag_ZF}, // Zero + {str_lit(\"s\"), ClobberFlag_SF}, // Sign + {str_lit(\"t\"), ClobberFlag_TF}, // Trap + {str_lit(\"i\"), ClobberFlag_IF}, // Interrupt Enable + {str_lit(\"d\"), ClobberFlag_DF}, // Direction + {str_lit(\"o\"), ClobberFlag_OF}, // Overflow + }; + + for (auto const &t : table) { + if (name == t.name) { + return cast(u16)t.flag; + } + } + return 0; + } + i32 flag_bit_from_name(String const &name, i32 *width_) { static const struct {String name; i32 bit; } table[] = { {str_lit(\"c\"), 0}, // Carry diff --git a/src/asm_tables_amd64.cpp b/src/asm_tables_amd64.cpp index 4fd0d03e6..657204e90 100644 --- a/src/asm_tables_amd64.cpp +++ b/src/asm_tables_amd64.cpp @@ -240,6 +240,27 @@ struct Asm_amd64 { return ""; } + u16 flag_from_name(String const &name) { + static const struct {String name; ClobberFlags flag; } table[] = { + {str_lit("c"), ClobberFlag_CF}, // Carry + {str_lit("p"), ClobberFlag_PF}, // Parity + {str_lit("a"), ClobberFlag_AF}, // Auxiliary Carry + {str_lit("z"), ClobberFlag_ZF}, // Zero + {str_lit("s"), ClobberFlag_SF}, // Sign + {str_lit("t"), ClobberFlag_TF}, // Trap + {str_lit("i"), ClobberFlag_IF}, // Interrupt Enable + {str_lit("d"), ClobberFlag_DF}, // Direction + {str_lit("o"), ClobberFlag_OF}, // Overflow + }; + + for (auto const &t : table) { + if (name == t.name) { + return cast(u16)t.flag; + } + } + return 0; + } + i32 flag_bit_from_name(String const &name, i32 *width_) { static const struct {String name; i32 bit; } table[] = { {str_lit("c"), 0}, // Carry diff --git a/src/asm_tables_riscv.cpp b/src/asm_tables_riscv.cpp index 62fddbda9..a118314a1 100644 --- a/src/asm_tables_riscv.cpp +++ b/src/asm_tables_riscv.cpp @@ -68,12 +68,12 @@ struct Asm_riscv { }; - enum ClobberFFlags : u8 { - ClobberFFlag_NV = 1<<0, // invalid operation - ClobberFFlag_DZ = 1<<1, // divide by zero - ClobberFFlag_OF = 1<<2, // overflow - ClobberFFlag_UF = 1<<3, // underflow - ClobberFFlag_NX = 1<<4, // inexact + enum ClobberFlags : u8 { + ClobberFlag_NV = 1<<0, // invalid operation + ClobberFlag_DZ = 1<<1, // divide by zero + ClobberFlag_OF = 1<<2, // overflow + ClobberFlag_UF = 1<<3, // underflow + ClobberFlag_NX = 1<<4, // inexact }; enum ClobberRegs : u8 { @@ -122,9 +122,28 @@ struct Asm_riscv { return ""; } + u16 flags_from_name(String const &name) { + static const struct { String name; ClobberFlags flag; } table[] = { + // flags: accrued FP exception flags (fcsr[4:0]) + {str_lit("nx"), ClobberFlag_NX}, // Inexact + {str_lit("uf"), ClobberFlag_UF}, // Underflow + {str_lit("of"), ClobberFlag_OF}, // Overflow + {str_lit("dz"), ClobberFlag_DZ}, // Divide by Zero + {str_lit("nv"), ClobberFlag_NV}, // Invalid Operation + }; + + for (auto const &t : table) { + if (name == t.name) { + return cast(u16)t.flag; + } + } + return 0; + } + + i32 flag_bit_from_name(String const &name, i32 *width_) { static const struct { String name; i32 bit; } table[] = { - // fflags: accrued FP exception flags (fcsr[4:0]) + // flags: accrued FP exception flags (fcsr[4:0]) {str_lit("nx"), 0}, // Inexact {str_lit("uf"), 1}, // Underflow {str_lit("of"), 2}, // Overflow @@ -155,14 +174,14 @@ struct Asm_riscv { OperandSet read; // operand slots whose register/CSR/mem-base is read ClobberRegs implicit_wr; // implicit reg writes (ra on C.JAL/C.JALR) ClobberRegs implicit_rd; // implicit reg reads (sp on the *SP forms) - ClobberFFlags fflags_wr; // accrued exception flags this op may raise + ClobberFlags flags_wr; // accrued exception flags this op may raise bool reads_frm; // consumes the dynamic rounding mode from fcsr bool writes_mem; bool reads_mem; SideEffectFlags side_effects; bool implies_clobber_flags() const { - return (fflags_wr != 0); + return (flags_wr != 0); } bool implies_clobber_memory() const { return writes_mem || reads_mem || diff --git a/src/check_asm.cpp b/src/check_asm.cpp index 1d5da9b02..3ed67ede8 100644 --- a/src/check_asm.cpp +++ b/src/check_asm.cpp @@ -1630,7 +1630,8 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm explicit_writes |= synth; } - facts->gen_regs = produced | pinned_param_writes; + facts->gen_regs = produced | pinned_param_writes; + facts->gen_flags = cast(u16)clobber.flags_wr; // NOTE(bill): mnemonics such as `xor r, r` / `sub r, r` act as zeroing the destination // independent of its prior value: the read is architecturally dead, so it must not count as a use. @@ -2715,7 +2716,7 @@ gb_internal void check_asm_template_from_entity(CheckerContext *c, Entity *e, De if (build_context.metrics.arch == TargetArch_amd64) { check_asm_template(&g_asm_amd64, c, e, d); } else if (build_context.metrics.arch == TargetArch_riscv64) { - check_asm_template(&g_asm_riscv, c, e, d); + // check_asm_template(&g_asm_riscv, c, e, d); } else { error(e->token, "asm templates are not currently supported for this target"); } diff --git a/src/check_asm_cfg.cpp b/src/check_asm_cfg.cpp index b866e1b56..3b55de62a 100644 --- a/src/check_asm_cfg.cpp +++ b/src/check_asm_cfg.cpp @@ -1,10 +1,16 @@ struct AsmBlock { i32 first, last; Array succs; + u16 in_defs; u16 out_defs; + + u16 in_flags; + u16 out_flags; + PtrSet in_params; PtrSet out_params; + bool reachable; }; @@ -12,6 +18,7 @@ struct AsmInstructionFacts { AstAsmInstruction *node; String name; + u16 gen_flags; // flag bits this instruction defines u16 gen_regs; u16 read_regs; @@ -105,6 +112,15 @@ gb_internal u16 asm_decl_resolve_pin_bit(AsmCtx *asm_ctx, Array +gb_internal u16 asm_decl_resolve_flag_bit(AsmCtx *asm_ctx, AsmTemplateEntityDecl const &ed) { + if (ed.pin_flag.len == 0) { + return 0; + } + auto flags = asm_ctx->flag_from_name(ed.pin_flag); + return cast(u16)flags; +} + template gb_internal void asm_cfg_populate_decls(AsmCtx *asm_ctx, AsmCfg *cfg, Entity *entity) { auto const &decls = entity->AsmTemplate.decls; @@ -305,7 +321,8 @@ gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, AsmCfg *cfg, CheckerCont error(entity->token, "'asm' templates cannot have more than 64 total parameter declarations, got %td", decls.count); return; } - u16 const REG_TOP = asm_ctx->CLOBBER_REGS_NAMED; + u16 const REG_TOP = asm_ctx->CLOBBER_REGS_NAMED; + u16 const FLAG_TOP = cast(u16)~cast(u16)0; u64 const universe_pm = cfg->universe_pm; auto bit_of = [&](Entity *e) -> u64 { @@ -338,12 +355,15 @@ gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, AsmCfg *cfg, CheckerCont isize const n = cfg->blocks.count; - auto in_regs = slice_make(heap_allocator(), n); defer (slice_free(&in_regs, heap_allocator())); - auto out_regs = slice_make(heap_allocator(), n); defer (slice_free(&out_regs, heap_allocator())); - auto gen_regs = slice_make(heap_allocator(), n); defer (slice_free(&gen_regs, heap_allocator())); - auto in_pm = slice_make(heap_allocator(), n); defer (slice_free(&in_pm, heap_allocator())); - auto out_pm = slice_make(heap_allocator(), n); defer (slice_free(&out_pm, heap_allocator())); - auto gen_pm = slice_make(heap_allocator(), n); defer (slice_free(&gen_pm, heap_allocator())); + auto in_regs = slice_make(heap_allocator(), n); defer (slice_free(&in_regs, heap_allocator())); + auto out_regs = slice_make(heap_allocator(), n); defer (slice_free(&out_regs, heap_allocator())); + auto gen_regs = slice_make(heap_allocator(), n); defer (slice_free(&gen_regs, heap_allocator())); + auto in_flags = slice_make(heap_allocator(), n); defer (slice_free(&in_flags, heap_allocator())); + auto out_flags = slice_make(heap_allocator(), n); defer (slice_free(&out_flags, heap_allocator())); + auto gen_flags = slice_make(heap_allocator(), n); defer (slice_free(&gen_flags, heap_allocator())); + auto in_pm = slice_make(heap_allocator(), n); defer (slice_free(&in_pm, heap_allocator())); + auto out_pm = slice_make(heap_allocator(), n); defer (slice_free(&out_pm, heap_allocator())); + auto gen_pm = slice_make(heap_allocator(), n); defer (slice_free(&gen_pm, heap_allocator())); // predecessors, restricted to reachable blocks auto preds = slice_make>(heap_allocator(), n); @@ -371,6 +391,7 @@ gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, AsmCfg *cfg, CheckerCont for_array(bi, cfg->blocks) { u16 gr = 0; + u16 gf = 0; u64 gp = 0; AsmBlock const &b = cfg->blocks[bi]; for (i32 ii = b.first; ii <= b.last; ii++) { @@ -379,12 +400,14 @@ gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, AsmCfg *cfg, CheckerCont continue; } gr |= f->gen_regs; + gf |= f->gen_flags; for (Entity *pe : f->gen_params) { gp |= bit_of(pe); } } - gen_regs[bi] = gr; - gen_pm[bi] = gp; + gen_regs[bi] = gr; + gen_flags[bi] = gf; + gen_pm[bi] = gp; } // NOTE(bill): initialize the blocks @@ -394,14 +417,17 @@ gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, AsmCfg *cfg, CheckerCont continue; } if (bi == 0) { - in_regs[bi] = seed_regs; - in_pm[bi] = seed_pm; + in_regs[bi] = seed_regs; + in_pm[bi] = seed_pm; + in_flags[bi] = 0; // no flag is defined at the template entry point } else { - in_regs[bi] = REG_TOP; - in_pm[bi] = universe_pm; + in_regs[bi] = REG_TOP; + in_pm[bi] = universe_pm; + in_flags[bi] = FLAG_TOP; } - out_regs[bi] = in_regs[bi] | gen_regs[bi]; - out_pm[bi] = in_pm[bi] | gen_pm[bi]; + out_regs[bi] = in_regs[bi] | gen_regs[bi]; + out_pm[bi] = in_pm[bi] | gen_pm[bi]; + out_flags[bi] = in_flags[bi] | gen_flags[bi]; } // forward must-analysis: in = AND(preds.out); out = in | gen. Iterate to fixpoint. @@ -414,26 +440,34 @@ gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, AsmCfg *cfg, CheckerCont } u16 nin_r = seed_regs; + u16 nin_f = 0; u64 nin_p = seed_pm; if (bi != 0) { nin_r = REG_TOP; + nin_f = FLAG_TOP; nin_p = universe_pm; for (i32 p : preds[bi]) { - nin_r &= out_regs[p]; - nin_p &= out_pm[p]; + nin_r &= out_regs[p]; + nin_r &= out_flags[p]; + nin_p &= out_pm[p]; } } u16 nout_r = nin_r | gen_regs[bi]; u64 nout_p = nin_p | gen_pm[bi]; + u16 nout_f = nin_f | gen_flags[bi]; if (nin_r != in_regs[bi] || nin_p != in_pm[bi] || + nin_f != in_flags[bi] || nout_r != out_regs[bi] || - nout_p != out_pm[bi]) { - in_regs[bi] = nin_r; - in_pm[bi] = nin_p; - out_regs[bi] = nout_r; - out_pm[bi] = nout_p; + nout_p != out_pm[bi] || + nout_f != out_flags[bi]) { + in_regs[bi] = nin_r; + in_pm[bi] = nin_p; + in_flags[bi] = nin_f; + out_regs[bi] = nout_r; + out_pm[bi] = nout_p; + out_flags[bi] = nout_f; changed = true; } } @@ -442,8 +476,10 @@ gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, AsmCfg *cfg, CheckerCont // NOTE(bill): publish the register masks and materialise the parameter sets onto the blocks for_array(bi, cfg->blocks) { AsmBlock *b = &cfg->blocks[bi]; - b->in_defs = in_regs[bi]; - b->out_defs = out_regs[bi]; + b->in_defs = in_regs[bi]; + b->out_defs = out_regs[bi]; + b->in_flags = in_flags[bi]; + b->out_flags = out_flags[bi]; if (!b->reachable) { continue; } @@ -529,8 +565,9 @@ gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, AsmCfg *cfg, CheckerCont } { // NOTE(bill): Collect the template's return points reachable blocks that leave via the end - u16 exit_regs = REG_TOP; - u64 exit_pm = universe_pm; + u16 exit_regs = REG_TOP; + u64 exit_pm = universe_pm; + u16 exit_flags = FLAG_TOP; bool any_exit = false; for_array(bi, cfg->blocks) { if (!cfg->blocks[bi].reachable) { @@ -540,8 +577,9 @@ gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, AsmCfg *cfg, CheckerCont continue; } any_exit = true; - exit_regs &= out_regs[bi]; - exit_pm &= out_pm[bi]; + exit_regs &= out_regs[bi]; + exit_pm &= out_pm[bi]; + exit_flags &= out_flags[bi]; } // NOTE(bill): Outputs must be assigned on every path that returns @@ -554,17 +592,18 @@ gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, AsmCfg *cfg, CheckerCont if (ed.tie >= 0 || ed.no_init) { continue; } - if (ed.pin_flag.len != 0) { - continue; // flag-pinned output: defined by a flags side effect, not a reg/param write - } bool written = false; - u16 bit = cfg->decl_pin_bit[i]; - if (bit != 0) { - written = (exit_regs & bit) != 0; + u16 flag_bit = asm_decl_resolve_flag_bit(asm_ctx, ed); + u16 reg_bit = cfg->decl_pin_bit[i]; + if (flag_bit != 0) { + // Flag-pinned output: defined iff the pinned flag is set on every returning path. + written = (exit_flags & flag_bit) != 0; + } else if (reg_bit != 0) { + written = (exit_regs & reg_bit) != 0; } else { written = (exit_pm & bit_of(ed.entity)) != 0; - } + } if (!written) { error(ed.entity->token, "'asm' output parameter '%.*s' is not assigned on all paths through this template; " From a9068261bfd45aa2cb8a5d969ac91c0f0697d197 Mon Sep 17 00:00:00 2001 From: gingerBill Date: Mon, 24 Aug 2026 23:12:38 +0100 Subject: [PATCH 17/33] Fix view-width testing for unpinned parameters in the lattice --- src/check_asm.cpp | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/check_asm.cpp b/src/check_asm.cpp index 3ed67ede8..b81864bc1 100644 --- a/src/check_asm.cpp +++ b/src/check_asm.cpp @@ -1671,6 +1671,34 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm if (cast(u16)clobber.written & (1u << slot)) { array_add(&facts->gen_params, pe); } + + + { // View aliasing: a view decl shares its source's physical register. + auto const &decls = tmpl_entity->AsmTemplate.decls; + i32 di = -1; + check_asm_find_group(pe, decls, &di); + if (di >= 0 && decls[di].view_of >= 0) { + i32 src_i = decls[di].view_of; + Entity *src_e = decls[src_i].entity; + if (src_e != nullptr) { + GB_ASSERT_MSG(asm_decl_resolve_pin_bit(asm_ctx, decls, cast(i32)di) == 0 && + asm_decl_resolve_pin_bit(asm_ctx, decls, src_i) == 0, + "view/source share a reg bit; fix the width gate on the reg-bit path, not gen_params"); + + // A read of the view is a read of the source + if (cast(u16)clobber.read & (1u << slot)) { + array_add(&facts->read_params, src_e); + } + // A write of the view defines the source only if it covers the parent + if (cast(u16)clobber.written & (1u << slot)) { + i32 parent_w = check_asm_operand_bit_width(src_e->type); + if (decls[di].view_bits == 32 || decls[di].view_bits == parent_w) { + array_add(&facts->gen_params, src_e); + } + } + } + } + } } { From e093329012f0cf39afc16a0f72b5c81f1b126f5f Mon Sep 17 00:00:00 2001 From: gingerBill Date: Mon, 24 Aug 2026 23:35:41 +0100 Subject: [PATCH 18/33] asm: add a backward liveness check for the CFG --- src/check_asm.cpp | 40 ++++++++---- src/check_asm_cfg.cpp | 140 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 166 insertions(+), 14 deletions(-) diff --git a/src/check_asm.cpp b/src/check_asm.cpp index b81864bc1..c03b081fd 100644 --- a/src/check_asm.cpp +++ b/src/check_asm.cpp @@ -1107,7 +1107,7 @@ gb_internal void check_operand_constraints(AsmCtx *asm_ctx, Slice const } template -gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tmpl_entity, AstAsmInstruction *instr, +gb_internal bool check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tmpl_entity, AstAsmInstruction *instr, u16 mnemonic, u16 pseudo_mnemonic, Slice const &operands, u8 previous_prefix, Ast *previous_prefix_instr, AsmCfg *cfg) { @@ -1514,7 +1514,7 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm error(instr->name, "The asm instruction '%.*s' expects %d..=%d operands, got %td", LIT(name), min_count, max_count, operands.count); } print_possible_forms(); - return; + return false; } if (matched) { @@ -1687,8 +1687,20 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm // A read of the view is a read of the source if (cast(u16)clobber.read & (1u << slot)) { - array_add(&facts->read_params, src_e); + i32 di = -1; + for_array(k, decls) { + if (decls[k].entity == pe) { + di = cast(i32)k; + break; + } + } + if (di >= 0 && decls[di].view_of >= 0 && decls[decls[di].view_of].entity != nullptr) { + array_add(&facts->read_params, decls[decls[di].view_of].entity); + } else { + array_add(&facts->read_params, pe); + } } + // A write of the view defines the source only if it covers the parent if (cast(u16)clobber.written & (1u << slot)) { i32 parent_w = check_asm_operand_bit_width(src_e->type); @@ -1766,8 +1778,10 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm cfg->impure_reason = why; cfg->impure_reason_node = instr->name; } + + // NOTE(bill): return true even on a purity test because the instruction is still good } - return; + return true; } // NOTE(bill): Failure path @@ -1878,6 +1892,8 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm print_possible_forms(); } end_error_block(); + + return false; } @@ -2461,6 +2477,7 @@ gb_internal void check_asm_template(AsmCtx *asm_ctx, CheckerContext *ctx, Entity u8 previous_prefix = 0; Ast *previous_prefix_instr = nullptr; // for a good error location + bool all_instructions_good = true; for (Ast *instruction_ : at->instructions) { switch (instruction_->kind) { @@ -2489,9 +2506,9 @@ gb_internal void check_asm_template(AsmCtx *asm_ctx, CheckerContext *ctx, Entity previous_prefix_instr = instruction_; } else if (res == CheckMnemomic_Mnemonic) { instr->suffix_flags = suffix_flags; - check_mnemonic(asm_ctx, ctx, entity, instr, mnemonic, 0, slice_from_array(operands), - previous_prefix, previous_prefix_instr, - &cfg); + all_instructions_good &= check_mnemonic(asm_ctx, ctx, entity, instr, mnemonic, 0, slice_from_array(operands), + previous_prefix, previous_prefix_instr, + &cfg); cfg.saw_any_instructions = true; @@ -2503,9 +2520,9 @@ gb_internal void check_asm_template(AsmCtx *asm_ctx, CheckerContext *ctx, Entity u16 pseudo_mnemonic = cast(u16)mnemonic; auto alias = asm_ctx->pseudo_alias(cast(u16)pseudo_mnemonic); u16 target_mnemonic = cast(u16)alias.target; - check_mnemonic(asm_ctx, ctx, entity, instr, target_mnemonic, pseudo_mnemonic, slice_from_array(operands), - previous_prefix, previous_prefix_instr, - &cfg); + all_instructions_good &=check_mnemonic(asm_ctx, ctx, entity, instr, target_mnemonic, pseudo_mnemonic, slice_from_array(operands), + previous_prefix, previous_prefix_instr, + &cfg); cfg.saw_any_instructions = true; @@ -2513,7 +2530,7 @@ gb_internal void check_asm_template(AsmCtx *asm_ctx, CheckerContext *ctx, Entity previous_prefix_instr = nullptr; } else if (res == CheckMnemomic_PseudoMacroMnemonic) { instr->suffix_flags = suffix_flags; - check_pseudo_macro_mnemonic(asm_ctx, entity, instr, slice_from_array(operands)); + all_instructions_good &= check_pseudo_macro_mnemonic(asm_ctx, entity, instr, slice_from_array(operands)); cfg.saw_any_instructions = true; @@ -2627,6 +2644,7 @@ gb_internal void check_asm_template(AsmCtx *asm_ctx, CheckerContext *ctx, Entity check_asm_cfg_build(asm_ctx, &cfg, d->init_expr, entity); check_asm_cfg_analyse(asm_ctx, &cfg, ctx, entity); + check_asm_cfg_liveness(asm_ctx, &cfg, entity, /*emit_dead_writes*/all_instructions_good); bool vet_unused = false; { diff --git a/src/check_asm_cfg.cpp b/src/check_asm_cfg.cpp index 3b55de62a..3cee17560 100644 --- a/src/check_asm_cfg.cpp +++ b/src/check_asm_cfg.cpp @@ -8,6 +8,9 @@ struct AsmBlock { u16 in_flags; u16 out_flags; + u16 live_in_regs; + u16 live_out_regs; + PtrSet in_params; PtrSet out_params; @@ -134,8 +137,18 @@ gb_internal void asm_cfg_populate_decls(AsmCtx *asm_ctx, AsmCfg *cfg, Entity *en Entity *e = decls[i].entity; cfg->decl_pin_bit[i] = asm_decl_resolve_pin_bit(asm_ctx, decls, cast(i32)i); if (e != nullptr) { - map_set(&cfg->entity_to_index, e, cast(i32)i); - cfg->universe_pm |= (cast(u64)1 << i); + if (decls[i].view_of >= 0) { + // NOTE(bill): A view shares its source's lattice bit, as it is not an independent value. + i32 src_i = decls[i].view_of; + Entity *src_e = decls[src_i].entity; + if (src_e != nullptr) { + map_set(&cfg->entity_to_index, e, src_i); // view entity -> source index + } + // NOTE(bill): No need to set a universe bit for the view as the source already has one + } else { + map_set(&cfg->entity_to_index, e, cast(i32)i); + cfg->universe_pm |= (cast(u64)1 << i); + } } } } @@ -549,8 +562,14 @@ gb_internal void check_asm_cfg_analyse(AsmCtx *asm_ctx, AsmCfg *cfg, CheckerCont if (((run_pm >> *ix) & 1) == 0 && !ptr_set_exists(&reported_params, pe)) { Ast *loc = instr->name; for (Ast *op : instr->operands) { - if (entity_of_node(op) == pe) { loc = op; break; } + if (entity_of_node(op) == pe) { + loc = op; + break; + } } + gb_printf_err("RBW-ERROR site=