Skip to content

Commit 62e2fcd

Browse files
committed
feat: Speculative discovery of nodes in memory
1 parent 4a4d5e8 commit 62e2fcd

182 files changed

Lines changed: 15028 additions & 5374 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

AGENTS.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@ dotnet test tests/Spice86.Tests --filter 'FullyQualifiedName!~SingleStepTest'
4848

4949
> **SingleStepTest exclusion rule**: `SingleStepTest` runs millions of CPU instruction test cases and take an extremely long time to complete. **Always exclude it** using `--filter 'FullyQualifiedName!~SingleStepTest'` unless the change being tested directly touches CPU instruction decoding, execution, or flag handling (e.g. changes to `CfgCpu`, instruction parsers, ALU operations, or flag computation). When in doubt, exclude it.
5050
51+
> **Test output rule (agents only)**: no need to redirect `dotnet test` output to a file. On success the console summary is short (a single `Passed! - Failed: 0, Passed: N ...` line), so run it directly and read the summary. Only capture to a file when a run actually fails and you need the full failure detail.
52+
53+
> **Long-running test rule (Kiro agents only)**: the foreground shell tool (`execute_bash`) enforces a hard wall-clock cap (roughly 25-60s in practice) that fires *before* the `timeout` argument. When it trips it force-returns exit 1 while the detached `dotnet` process keeps running and completes normally, which looks exactly like a crash (abrupt exit 1, no test summary, no dump) but is not one. Any test run expected to exceed ~25s (the full suite is ~2 min; `CfgGraphReloadTest` alone is ~55s) MUST be launched with the background-process tool (which has no such cap), writing a completion marker, then polled with short (<20s) foreground calls until the marker appears. Do not conclude a suite "crashed" from a bare exit 1 with no summary; re-run it in the background and check the real result first. Splitting `--filter` so each foreground run stays under the cap also works.
54+
5155
### Debugging Workflow
5256
- **GDB Integration**: Server runs on port 10000 by default (`--GdbPort 10000`)
5357
- Use `--Debug` to pause at startup for breakpoint setup
@@ -117,9 +121,10 @@ Variants: `MemoryBasedDataStructureWithCsBaseAddress`, `MemoryBasedDataStructure
117121
- **No scripts outside the project** - do NOT create temporary scripts (Python, Bash, PowerShell, Node, etc.) anywhere on the filesystem, including `/tmp`, the user's home directory, or any location outside the workspace. Do not create throwaway helper scripts inside the workspace either. Use the available tools (file editing, grep, search, terminal one-liners) directly. If a multi-step computation is truly needed, run it as an inline shell one-liner in the terminal without writing a file.
118122
- **Use `tmp/` for temporary files** - if a temporary file must be written (e.g., captured command output, intermediate data), place it inside the `tmp/` folder at the root of the repository. Never write to `/tmp` or any path outside the workspace.
119123
- **Avoid complexity** - keep cyclomatic complexity low, prefer simple, linear code over nested conditionals
120-
- **No optional parameters** - avoid nullable or optional parameters in new code
124+
- **No optional parameters** - avoid optional/defaulted method parameters in new code. Prefer non-nullable types; a field or dependency should be nullable only when its absence is a real, distinct state (e.g. a feature disabled by a flag). When a nullable feature-gate would otherwise force `?.`/`is not null` guards across many call sites, prefer a null-object (no-op) implementation or unconditional construction over a nullable reference.
121125
- **No complex ternary expressions** - avoid nested, chained, or multi-line ternaries; simple single-line ternaries with short operands are allowed (e.g. `int x = a > b ? a : b;`), but non-trivial conditions or non-trivial branches must use explicit `if/else`
122126
- **Minimal comments** - write self-documenting code with clear names; avoid obvious comments
127+
- **No references to plan/spec sections in code** - never point comments, XML docs, or test names at sections of a plan, spec, or implementation-tracking document (e.g. "Phase 5", "Gap D", "Rule 4", "section S6.1.1", "Test 18", "implementation plan"). Those documents are temporary or drift over time, leaving dangling pointers that mean nothing to a future reader. Describe the actual behavior, invariant, or scenario directly instead.
123128
- **Test before submit** - always run tests after code changes to verify functionality
124129
- **Rebuild and verify** - For any task that changes code or tests, rebuild the project and run the full test suite; do not stop until all tests are green.
125130
- **Concise documentation** - XML docs should be precise and complete but not verbose; avoid excessive remarks

src/Spice86.Core/CLI/Configuration.cs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,4 +357,17 @@ public sealed class Configuration : CommandSettings {
357357
[CommandOption("--AllowIvtAddress0")]
358358
public bool AllowIvtAddress0 { get; init; }
359359

360+
/// <summary>
361+
/// When true (default), the speculative CFG explorer is enabled: after each newly-observed
362+
/// instruction is parsed, the explorer performs a recursive-descent static decode of all
363+
/// statically-reachable successors that have not yet been observed, marking the resulting nodes
364+
/// as speculative. This allows the C# code generator to emit real branches for unobserved
365+
/// conditional arms instead of always falling back to <c>FailAsUntested</c>.
366+
/// When false, no speculative nodes, edges, or index entries are created; the graph content is
367+
/// byte-identical to the pre-feature baseline.
368+
/// </summary>
369+
[CommandOption("--EnableSpeculativeCfgExploration <ENABLESPECULATIVECFGEXPLORATION>")]
370+
[DefaultValue(true)]
371+
public bool EnableSpeculativeCfgExploration { get; init; } = true;
372+
360373
}

src/Spice86.Core/Emulator/CPU/CfgCpu/CfgCpu.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,14 @@ public CfgCpu(IMemory memory, State state, IOPortDispatcher ioPortDispatcher, Ca
3434
DualPic dualPic, EmulatorBreakpointsManager emulatorBreakpointsManager,
3535
IPauseHandler pauseHandler,
3636
FunctionCatalogue functionCatalogue,
37-
bool useCodeOverride, bool failOnInvalidOpcode, bool allowIvtAddress0, ILoggerService loggerService, CfgNodeExecutionCompiler executionCompiler, SequentialIdAllocator idAllocator, CpuHeavyLogger? cpuHeavyLogger = null) {
37+
bool useCodeOverride, bool failOnInvalidOpcode, bool allowIvtAddress0, bool enableSpeculativeExploration, ILoggerService loggerService, CfgNodeExecutionCompiler executionCompiler, SequentialIdAllocator idAllocator, CpuHeavyLogger? cpuHeavyLogger = null) {
3838
_loggerService = loggerService;
3939
_state = state;
4040
_dualPic = dualPic;
4141
_cpuHeavyLogger = cpuHeavyLogger;
4242
_emulatorBreakpointsManager = emulatorBreakpointsManager;
4343
_pauseHandler = pauseHandler;
44-
CfgNodeFeeder = new(memory, state, emulatorBreakpointsManager, _replacerRegistry, executionCompiler, idAllocator);
44+
CfgNodeFeeder = new(memory, state, emulatorBreakpointsManager, _replacerRegistry, executionCompiler, idAllocator, enableSpeculativeExploration);
4545
_executionContextManager = new(memory, state, CfgNodeFeeder, _replacerRegistry, functionCatalogue, useCodeOverride, loggerService, cpuHeavyLogger);
4646
_instructionExecutionHelper = new(state, memory, ioPortDispatcher, callbackHandler, emulatorBreakpointsManager, _executionContextManager, failOnInvalidOpcode, allowIvtAddress0, loggerService);
4747
}
@@ -61,6 +61,7 @@ public CfgCpu(IMemory memory, State state, IOPortDispatcher ioPortDispatcher, Ca
6161
public FunctionHandler FunctionHandlerInUse => ExecutionContextManager.CurrentExecutionContext.FunctionHandler;
6262
public bool IsInitialExecutionContext => ExecutionContextManager.CurrentExecutionContext.Depth == 0;
6363
private ExecutionContext CurrentExecutionContext => _executionContextManager.CurrentExecutionContext;
64+
6465
public ICfgNode ToExecute() {
6566
return CfgNodeFeeder.GetLinkedCfgNodeToExecute(CurrentExecutionContext);
6667
}

src/Spice86.Core/Emulator/CPU/CfgCpu/ControlFlowGraph/CfgBlock.cs

Lines changed: 67 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@ public sealed class CfgBlock : CfgNode {
2323
/// </summary>
2424
private int _nonLiveCounter;
2525

26+
/// <summary>
27+
/// Number of contained nodes whose <see cref="ICfgNode.IsSpeculative"/> is currently <c>true</c>.
28+
/// <see cref="IsSpeculative"/> is <c>(_speculativeCounter &gt; 0)</c>.
29+
/// </summary>
30+
private int _speculativeCounter;
31+
2632
private BlockNode? _cachedDisplayAst;
2733
private bool _isDisplayAstStale = true;
2834

@@ -50,6 +56,13 @@ public CfgBlock(int id, ICfgNode entry)
5056
/// </remarks>
5157
public override bool IsLive => _nonLiveCounter == 0;
5258

59+
/// <inheritdoc />
60+
/// <remarks>
61+
/// O(1). The block is speculative iff at least one contained instruction is speculative;
62+
/// computed from the maintained <see cref="_speculativeCounter"/> without iterating.
63+
/// </remarks>
64+
public override bool IsSpeculative => _speculativeCounter > 0;
65+
5366
/// <inheritdoc />
5467
/// <remarks>
5568
/// Always <c>null</c>: a <see cref="CfgBlock"/> is itself the container, not contained in one.
@@ -140,19 +153,22 @@ public override IVisitableAstNode DisplayAst {
140153

141154
/// <summary>
142155
/// Appends <paramref name="node"/> to the end of the block, making it the new
143-
/// <see cref="Terminator"/>. Updates the non-live counter from the node's live state.
156+
/// <see cref="Terminator"/>. Updates the non-live and speculative counters from the node's state.
144157
/// </summary>
145158
internal void Append(ICfgNode node) {
146159
_instructions.Add(node);
147160
if (!node.IsLive) {
148161
_nonLiveCounter++;
149162
}
163+
if (node.IsSpeculative) {
164+
_speculativeCounter++;
165+
}
150166
_isDisplayAstStale = true;
151167
}
152168

153169
/// <summary>
154170
/// Replaces the node at <paramref name="index"/> with <paramref name="newNode"/>, preserving
155-
/// order. Adjusts the non-live counter to reflect the new node's live state.
171+
/// order. Adjusts the non-live and speculative counters to reflect the new node's state.
156172
/// </summary>
157173
internal void ReplaceInPlace(int index, ICfgNode newNode) {
158174
ICfgNode old = _instructions[index];
@@ -163,18 +179,46 @@ internal void ReplaceInPlace(int index, ICfgNode newNode) {
163179
if (!newNode.IsLive) {
164180
_nonLiveCounter++;
165181
}
182+
if (old.IsSpeculative) {
183+
_speculativeCounter--;
184+
}
185+
if (newNode.IsSpeculative) {
186+
_speculativeCounter++;
187+
}
166188
_isDisplayAstStale = true;
167189
}
168190

191+
/// <summary>
192+
/// Removes <paramref name="node"/> from the block by value, maintaining the non-live and
193+
/// speculative counters via O(1) incremental decrement (mirrors <see cref="ReplaceInPlace"/>).
194+
/// Returns <c>false</c> when the node is not part of the block. Tolerates transient
195+
/// non-contiguity: it is a batch-internal primitive for removing a contiguous suffix of
196+
/// nodes one at a time.
197+
/// </summary>
198+
internal bool Remove(ICfgNode node) {
199+
if (!_instructions.Remove(node)) {
200+
return false;
201+
}
202+
if (!node.IsLive) {
203+
_nonLiveCounter--;
204+
}
205+
if (node.IsSpeculative) {
206+
_speculativeCounter--;
207+
}
208+
_isDisplayAstStale = true;
209+
return true;
210+
}
211+
169212
/// <summary>
170213
/// Removes nodes from <paramref name="splitIndex"/> through the end and returns them as a
171-
/// new list. The non-live counter is recomputed for the surviving prefix.
214+
/// new list. The non-live and speculative counters are recomputed for the surviving prefix.
172215
/// </summary>
173216
internal List<ICfgNode> SliceFrom(int splitIndex) {
174217
int tailLength = _instructions.Count - splitIndex;
175218
List<ICfgNode> tail = _instructions.GetRange(splitIndex, tailLength);
176219
_instructions.RemoveRange(splitIndex, tailLength);
177220
RecountNonLiveFromInstructions();
221+
RecountSpeculativeFromInstructions();
178222
_isDisplayAstStale = true;
179223
return tail;
180224
}
@@ -191,6 +235,18 @@ internal void OnContainedInstructionLiveChanged(bool nowLive) {
191235
}
192236
}
193237

238+
/// <summary>
239+
/// Counter choke-point invoked by <see cref="CfgInstruction.SetSpeculative"/> exactly once per
240+
/// actual <see cref="ICfgNode.IsSpeculative"/> transition of a contained instruction.
241+
/// </summary>
242+
internal void OnContainedInstructionSpeculativeChanged(bool nowSpeculative) {
243+
if (nowSpeculative) {
244+
_speculativeCounter++;
245+
} else {
246+
_speculativeCounter--;
247+
}
248+
}
249+
194250
/// <summary>
195251
/// Recomputes the non-live counter from scratch by iterating the contained instructions.
196252
/// Used after a split to re-base the counter on the new instruction set.
@@ -199,6 +255,14 @@ internal void RecountNonLiveFromInstructions() {
199255
_nonLiveCounter = _instructions.Count(node => !node.IsLive);
200256
}
201257

258+
/// <summary>
259+
/// Recomputes the speculative counter from scratch by iterating the contained instructions.
260+
/// Used after a split to re-base the counter on the new instruction set.
261+
/// </summary>
262+
internal void RecountSpeculativeFromInstructions() {
263+
_speculativeCounter = _instructions.Count(node => node.IsSpeculative);
264+
}
265+
202266
/// <summary>
203267
/// Returns the index of <paramref name="node"/> in the contained list, or <c>-1</c> if
204268
/// the node is not part of this block.

src/Spice86.Core/Emulator/CPU/CfgCpu/ControlFlowGraph/CfgNode.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,21 @@ public CfgNodeExecutionAction<InstructionExecutionHelper> CompiledExecution {
4343
}
4444

4545
public abstract bool IsLive { get; }
46+
47+
/// <summary>
48+
/// Default is <c>false</c>. <see cref="ParsedInstruction.CfgInstruction"/> overrides this to
49+
/// expose its stored speculative flag, maintained by <see cref="SetSpeculative"/> calls.
50+
/// <see cref="SelectorNode"/> and <see cref="CfgBlock"/> keep the default <c>false</c>.
51+
/// </summary>
52+
public virtual bool IsSpeculative => false;
53+
54+
/// <summary>
55+
/// No-op default: <see cref="SelectorNode"/> and <see cref="CfgBlock"/> have no speculative
56+
/// state. <see cref="ParsedInstruction.CfgInstruction"/> overrides this to maintain its stored
57+
/// flag and notify its containing block.
58+
/// </summary>
59+
public virtual void SetSpeculative(bool isSpeculative) {
60+
}
4661

4762
public abstract void UpdateSuccessorCache();
4863
public abstract ICfgNode? GetNextSuccessor(InstructionExecutionHelper helper);

src/Spice86.Core/Emulator/CPU/CfgCpu/ControlFlowGraph/ICfgNode.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,24 @@ public interface ICfgNode : IEquatable<ICfgNode> {
3737
/// </summary>
3838
bool IsLive { get; }
3939

40+
/// <summary>
41+
/// Returns whether the node is speculative.
42+
/// A speculative node was decoded by the static explorer from memory bytes but has never been
43+
/// executed and confirmed. It is a separate axis from <see cref="IsLive"/>: a speculative node
44+
/// is always non-live, but a non-live observed node (e.g. SMC-evicted) is not speculative.
45+
/// Speculative nodes never enter <see cref="Feeder.CurrentInstructions"/>,
46+
/// <see cref="Feeder.PreviousInstructions"/>, or a <see cref="ParsedInstruction.SelfModifying.SelectorNode"/>.
47+
/// </summary>
48+
bool IsSpeculative { get; }
49+
50+
/// <summary>
51+
/// Updates the speculative provenance of this node. On an actual transition, keeps the
52+
/// containing <see cref="CfgBlock"/>'s speculative counter in sync. Setting speculative also
53+
/// forces the node non-live. No-op for node kinds that have no speculative state
54+
/// (<see cref="ParsedInstruction.SelfModifying.SelectorNode"/>, <see cref="CfgBlock"/>).
55+
/// </summary>
56+
void SetSpeculative(bool isSpeculative);
57+
4058
/// <summary>
4159
/// True when the node execution can lead to going back to previous execution context if the next to execute is the correct address
4260
/// </summary>

src/Spice86.Core/Emulator/CPU/CfgCpu/ExecutionContextManager.cs

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@
1111
using Spice86.Core.Emulator.Memory;
1212
using Spice86.Shared.Emulator.Memory;
1313
using Spice86.Shared.Interfaces;
14+
using Spice86.Shared.Utils;
15+
16+
using System.Linq;
1417

1518
public class ExecutionContextManager : InstructionReplacer, IClearable {
1619
private readonly ILoggerService _loggerService;
@@ -121,6 +124,39 @@ private void RegisterCurrentInstructionAsEntryPoint(SegmentedAddress entryAddres
121124
nodes.Add(toExecute);
122125
}
123126

127+
/// <summary>
128+
/// Registers <paramref name="node"/> as a CFG entry point so it is treated as a generation root by
129+
/// the graph exporter and the function partitioner. Used to seed emulator-installed hardware
130+
/// interrupt handlers: these fire on external events with nondeterministic timing and may never be
131+
/// reached from the program's observed entry points during discovery, yet must still become
132+
/// generated overrides so generated code can service the interrupt.
133+
/// </summary>
134+
/// <param name="node">The handler entry instruction to register.</param>
135+
public void RegisterEntryPoint(CfgInstruction node) {
136+
if (!ExecutionContextEntryPoints.TryGetValue(node.Address, out ISet<CfgInstruction>? nodes)) {
137+
nodes = new HashSet<CfgInstruction>();
138+
ExecutionContextEntryPoints.Add(node.Address, nodes);
139+
}
140+
nodes.Add(node);
141+
}
142+
143+
/// <summary>
144+
/// Seeds the given known-safe handler entry addresses for speculative exploration and registers
145+
/// each decoded handler entry node as a CFG entry point. No-op per handler when speculative
146+
/// exploration is disabled (no seeded node is produced).
147+
/// </summary>
148+
/// <param name="handlerAddresses">Entry addresses of emulator-installed interrupt handlers.</param>
149+
public void SeedKnownSafeHandlersAndRegisterEntryPoints(IReadOnlyList<SegmentedAddress> handlerAddresses) {
150+
_cfgNodeFeeder.SeedKnownSafeHandlers(handlerAddresses);
151+
foreach (SegmentedAddress handlerAddress in handlerAddresses) {
152+
CfgInstruction? entryNode = _cfgNodeFeeder.NodeIndex.GetAtAddress(handlerAddress)
153+
.FirstOrDefault(node => node.ContainingBlock is not null);
154+
if (entryNode is not null) {
155+
RegisterEntryPoint(entryNode);
156+
}
157+
}
158+
}
159+
124160
public override void ReplaceInstruction(CfgInstruction oldInstruction, CfgInstruction newInstruction) {
125161
if (ExecutionContextEntryPoints.TryGetValue(newInstruction.Address, out ISet<CfgInstruction>? entriesAtAddress)
126162
&& entriesAtAddress.Remove(oldInstruction)) {
@@ -132,6 +168,24 @@ public override void ReplaceInstruction(CfgInstruction oldInstruction, CfgInstru
132168
}
133169
}
134170

171+
/// <summary>
172+
/// Handles removal fan-out: drops <paramref name="instruction"/> from the
173+
/// entry-point set at its address so a removed generation root cannot linger as a
174+
/// detached, de-indexed dead root. Removes the address key when its set empties.
175+
/// </summary>
176+
/// <remarks>
177+
/// Deliberately does NOT clear a context's <see cref="ExecutionContext.NodeToExecuteNextAccordingToGraph"/>
178+
/// that still points at the removed node, unlike <see cref="ReplaceInstruction"/> which repoints it.
179+
/// Replacement has a live successor to point at; removal does not. The feeder's
180+
/// reconcile-with-memory path relies on the stale pointer surviving the sweep: a non-live graph
181+
/// node whose address still matches memory but is no longer indexed is how it detects a swept
182+
/// speculative node and routes to the live memory node. Nulling it here makes that node null and
183+
/// trips the address-mismatch guard instead.
184+
/// </remarks>
185+
public override void RemoveInstruction(CfgInstruction instruction) {
186+
DictionaryUtils.RemoveFromCollection(ExecutionContextEntryPoints, instruction.Address, instruction);
187+
}
188+
135189
private static void UpdateNodeToExecuteIfStale(ExecutionContext context,
136190
CfgInstruction oldInstruction, CfgInstruction newInstruction) {
137191
if (oldInstruction.Equals(context.NodeToExecuteNextAccordingToGraph)) {

0 commit comments

Comments
 (0)