Refactor bisector system, add supervisor, add unit tests, and fix transformations - #1290
Merged
Merged
Conversation
…nsformations. Major refactor of UnitTestBisectorSystem: introduces new bisector plans (DisableOne, EnableOne, RandomCombo, FailureInducing, Masking), robust state management, improved transform observation, and enhanced reporting. Adds Mosa.Utility.UnitTestBisector.Supervisor for process supervision and restart. Updates command-line options, documentation, and project files. Improves diagnostics, output, and testability throughout the bisector and unit test infrastructure.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the unit test bisector tooling to support resumable “plans” and introduces a supervisor process to run bisector iterations in restarted child processes, while also fixing/adjusting several compiler transformations and expanding xUnit coverage for bit-tracking logic.
Changes:
- Refactors
UnitTestBisectorSysteminto multiple plans with JSON state persistence and improved transform observation/reporting. - Adds
Mosa.Utility.UnitTestBisector.Supervisorplus new configuration/CLI options and documentation updates. - Fixes/updates several transforms (x64
Not64, strength-reduction patterns, constant-folding convert opcodes) and adds new xUnit tests.
Reviewed changes
Copilot reviewed 52 out of 55 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| Source/Mosa.sln | Adds the supervisor project to the main solution. |
| Source/Mosa.Linux.sln | Adds bisector + supervisor projects to the Linux solution. |
| Source/Mosa.Workspace.Experiment.Debug/Program.cs | Updates event filtering to use IsStandardNotifyEvent. |
| Source/Mosa.Utility.UnitTests/UnitTestEngine.cs | Improves error capture/reporting and unit test progress output. |
| Source/Mosa.Utility.UnitTests/Mosa.Utility.UnitTests.csproj | Adjusts runtime config generation/self-contained settings. |
| Source/Mosa.Utility.UnitTestBisector/UnitTestBisectorSystem.cs | Major refactor: plans, state persistence, transform observation, reporting. |
| Source/Mosa.Utility.UnitTestBisector/Program.cs | Formatting-only changes; still registers platforms directly. |
| Source/Mosa.Utility.UnitTestBisector/PlanResult.cs | Adds a nested plan result model for persisted reporting. |
| Source/Mosa.Utility.UnitTestBisector/BisectorState.cs | Adds persisted state model for resumable runs. |
| Source/Mosa.Utility.UnitTestBisector/AssertFailureException.cs | Adds exception type used to treat Debug.Assert as failures. |
| Source/Mosa.Utility.UnitTestBisector/AssertExceptionTraceListener.cs | Adds trace listener that converts asserts to exceptions. |
| Source/Mosa.Utility.UnitTestBisector/AssertCaptureScope.cs | Adds scope for temporarily disabling assert UI and installing listener. |
| Source/Mosa.Utility.UnitTestBisector/Mosa.Utility.UnitTestBisector.csproj | Formatting changes in project file. |
| Source/Mosa.Utility.UnitTestBisector.Supervisor/Program.cs | New supervisor app entry point. |
| Source/Mosa.Utility.UnitTestBisector.Supervisor/ProcessSupervisor.cs | Implements restart/continue supervision using state snapshots. |
| Source/Mosa.Utility.UnitTestBisector.Supervisor/Mosa.Utility.UnitTestBisector.Supervisor.csproj | New supervisor project file. |
| Source/Mosa.Utility.Launcher/Builder.cs | Adjusts output formatting; introduces an unused JavaScript-related import. |
| Source/Mosa.Utility.CreateCoreLib/Mosa.Utility.CreateCoreLib.csproj | Bumps ICSharpCode.Decompiler package version. |
| Source/Mosa.Utility.Configuration/Name.cs | Adds new bisector/supervisor setting keys. |
| Source/Mosa.Utility.Configuration/MOSASettings.cs | Adds bisector/supervisor settings + app location for bisector; updates app locations hook. |
| Source/Mosa.Utility.Configuration/CommandLineArguments.cs | Adds new bisector/supervisor CLI flags + loop range tracker flags in -o presets. |
| Source/Mosa.Utility.Configuration/AppLocationsSettings.cs | Removes legacy app-locations implementation. |
| Source/Mosa.Utility.Configuration/AppLocations.cs | Adds replacement app-locations implementation + bisector target discovery. |
| Source/Mosa.Utility.BootImage/Mosa.Utility.BootImage.csproj | Bumps System.IO.Hashing package version. |
| Source/Mosa.UnitTests/Optimization/ComplexTests.cs | Adds new bare-metal unit tests in Mosa.UnitTests. |
| Source/Mosa.Tool.Launcher/Mosa.Tool.Launcher.csproj | Bumps Avalonia/Tmds.DBus.Protocol packages. |
| Source/Mosa.Tool.Explorer/Mosa.Tool.Explorer.csproj | Bumps System.ComponentModel.Composition package version. |
| Source/Mosa.Tool.Explorer.Avalonia/Mosa.Tool.Explorer.Avalonia.csproj | Bumps Avalonia/Tmds.DBus.Protocol packages. |
| Source/Mosa.Tool.Compiler/Compiler.cs | Updates event filtering + adds Windows affinity reporting. |
| Source/Mosa.Tool.Bootstrap/Mosa.Tool.Bootstrap.csproj | Bumps Avalonia/Tmds.DBus.Protocol packages. |
| Source/Mosa.Compiler.x64/Transforms/BaseIR/Not64.cs | Fixes Not64 lowering to emit NOT instead of MOV. |
| Source/Mosa.Compiler.Framework/Transforms/Optimizations/Auto/StrengthReduction/Compare64x64RemUnsigned.cs | Updates strength-reduction result pattern to include NOT + AND. |
| Source/Mosa.Compiler.Framework/Transforms/Optimizations/Auto/StrengthReduction/Compare64x32RemUnsigned.cs | Updates strength-reduction result pattern to include NOT + AND. |
| Source/Mosa.Compiler.Framework/Transforms/Optimizations/Auto/StrengthReduction/Compare32x64RemUnsigned.cs | Updates strength-reduction result pattern to include NOT + AND. |
| Source/Mosa.Compiler.Framework/Transforms/Optimizations/Auto/StrengthReduction/Compare32x32RemUnsigned.cs | Updates strength-reduction result pattern to include NOT + AND. |
| Source/Mosa.Compiler.Framework/Transforms/Optimizations/Auto/ConstantFolding/ConvertU32ToR8.cs | Fixes transform opcode from I32->R8 to U32->R8. |
| Source/Mosa.Compiler.Framework/Transforms/Optimizations/Auto/ConstantFolding/ConvertU32ToR4.cs | Fixes transform opcode from I32->R4 to U32->R4. |
| Source/Mosa.Compiler.Framework/Stages/BaseTransformStage.cs | Extends transform observation callback to include method full name. |
| Source/Mosa.Compiler.Framework/CompilerHooks.cs | Updates transform observation delegate signature; replaces notify-event filter helper. |
| Source/Mosa.Compiler.Framework/Compiler.cs | Comments out safepoint stages in the method pipeline. |
| Source/Mosa.Compiler.Framework/BitValue.cs | Adds IsSame helper and tests. |
| Source/Mosa.Compiler.Framework.xUnit/Mosa.Compiler.Framework.xUnit.csproj | Updates test SDK package version. |
| Source/Mosa.Compiler.Framework.xUnit/BitValueTests.cs | Adds tests for BitValue.IsSame. |
| Source/Mosa.Compiler.Framework.xUnit/BitTrackerOperationsTests.cs | Adds extensive new tests for bit tracker operations. |
| Source/Mosa.Compiler.Framework.xUnit/BisectorTests.cs | Enables pairwise mode in relevant tests. |
| Source/Mosa.Compiler.Common.xUnit/Mosa.Compiler.Common.xUnit.csproj | Updates test SDK package version. |
| Source/Docs/unit-tests.rst | Adds bisector and supervisor documentation/usage examples. |
| Source/Docs/settings-options.rst | Documents new bisector/supervisor settings + app location. |
| Source/Docs/command-line-arguments.rst | Updates argument documentation and adds bisector/supervisor flags. |
| Source/Data/IR-Optimizations-StrengthReduction-Complex.json | Updates generated strength-reduction result pattern. |
| Source/Data/IR-Optimizations-ConstantFolding.json | Updates generated constant-folding expression for U32->R#. |
| Source/.github/copilot-instructions.md | Adds a reminder to update Docs when adding new settings/CLI. |
| .gitignore | Adds temp dirs; appears to include an accidental false entry. |
Comments suppressed due to low confidence (1)
Source/Docs/unit-tests.rst:86
- This example uses
Mosa.Utility.UnitTestBisector.Supervisor.exeand-bisect-state, but on non-Windows the supervisor will be a.dlland the implemented flag is-bisect-state-file. Please update the example to a cross-platform invocation (e.g.,dotnet ...Supervisor.dll ...) and use the correct flag (or document the alias if added).
Use ``Mosa.Utility.UnitTestBisector.Supervisor`` to run one bisector worker iteration per child process and automatically restart until completion.
.. code-block:: bash
dotnet bin/Mosa.Utility.UnitTestBisector.Supervisor.exe -bisect -bisect-state artifact/bisect-state.json -bisect-worker-iteration
Activated SafePointStage before and SafePointLayoutStage after CodeGenerationStage in the method compiler pipeline, enabling GC safepoint infrastructure for improved garbage collection support.
- Renames `-bisect-state-file` to `-bisect-state` for consistency. - Refactors bisector plan/order parsing to use switch expressions. - Simplifies output and reporting in bisector system. - Removes redundant/debug code from transform disabling logic. - Updates `Mosa.Linux.sln` for VS2022 and normalizes configs. - Applies minor string, argument, and formatting cleanups.
Move magic constants (exit kinds, baseline iteration, option names, etc.) to a new Constant.cs file. Update all usages in BisectorState.cs and UnitTestBisectorSystem.cs to reference the Constant class, improving maintainability and reducing duplication.
Eliminates all handling of forced-disabled transforms and the related file option from the bisector system. Updates state, compatibility checks, and reporting to only use session-based disabled transforms. Refactors IsSupervisorOption to use a switch expression and performs minor cleanup in unit test handling and status output.
Refactored private nested classes in Mosa.Utility.UnitTestBisector to internal top-level classes for better accessibility and maintainability. Moved PlanKind and OrderKind enums to separate files. Removed BisectorDisabledTransformsFile from MOSASettings. No functional changes.
Extract unit test serialization, result parsing, and formatting logic from UnitTestSystem into a new static UnitTestSerializer class. Update all usages in UnitTestSystem, UnitTestEngine, and UnitTestBisectorSystem to use UnitTestSerializer, improving code organization and separation of concerns.
Centralize unit test discovery and execution in a new UnitTestRunner static class. Update UnitTestBisectorSystem and UnitTestSystem to use this runner, passing discovered tests explicitly. Remove shared state for test lists, clean up unused code, and improve separation of concerns for better maintainability.
Clarified Sub64 by adding an explicit else branch. Now, when value2 is zero, the result is narrowed to value1; otherwise, SetStable(value1, value2) is used. This improves code readability and intent.
Added thorough unit tests for SignExtend32x64 and SignExtend8x64, covering all sign bit and known/unknown bit scenarios. Fixed SignExtend32x64 to use the correct upper 32 bits mask for sign extension.
Added default settings for ReduceCodeSize and inlining options in MOSASettings. Reformatted Constant.cs to use tabs for indentation. No logic changes in constants.
Refactored bisector logic to introduce explicit session state fields in BisectorState, persist session name and invert outcome, and improve masking pre-check handling. Platform registration is now centralized. Bisector plan execution is split into clear phases with robust state management, supporting single-iteration-per-process and supervisor restarts. Output and disabled transform logic are clarified and simplified.
Previously, the loop skipped the first result when restoring bisector state. Now, all results in state.Results are replayed to ensure the bisector state is fully restored.
Comprehensive regression and additional unit tests were added for BitTrackerOperations. Fixed Add64 to only narrow min/max when no unsigned overflow is possible. Corrected And32 to use BitsSet32 for zero checks. Updated RemUnsigned32/64 to use value2.MaxValue for bounds and consolidated divide-by-zero guards. Fixed ShiftRight64 to track stability with both operands.
Added comprehensive unit tests for Add32, MulUnsigned32, MulSigned32, MulSigned64, and To64, covering overflow and range-narrowing scenarios. Simplified Add32 overflow checks by removing unreachable conditions. Corrected MulSigned32 and MulSigned64 to use actual min/max products for range narrowing. Fixed MulUnsigned32 to only clear upper 32 bits on overflow. Clarified RemUnsigned32 zero-divisor comment. Updated To64 to use BitsSet32 for constructing 64-bit values from 32-bit parts.
Implemented AddCarryOut32 and AddCarryOut64 in BitTrackerOperations for precise sum and carry-out tracking. Updated BitTrackerStage to use these handlers for IR.AddCarryOut32/64. Added thorough tests for Result2NarrowToBoolean, covering upper bits, unconstrained, known true/false, and stability cases.
Removed default stage restriction in bisector logic, allowing bisecting across all stages when BisectorStage is unset. Output now displays "All" for unspecified stages. Also removed unused upper bit mask constants from BitTrackerStage.cs.
Comment on lines
+8
to
+18
| public static class UnitTestSerializer | ||
| { | ||
| public static List<int> SerializeUnitTestMessage(UnitTest unitTest) | ||
| { | ||
| var address = unitTest.MosaMethodAddress.ToInt64(); | ||
|
|
||
| var cmd = new List<int>(4 + 4 + 4 + 4 + unitTest.MosaMethod.Signature.Parameters.Count) | ||
| { | ||
| (int)address, | ||
| GetReturnResultType(unitTest.MosaMethod.Signature.ReturnType), | ||
| 0 |
Replaced SubCarryOut32 and SubCarryOut64 handlers with the generic Result2NarrowToBoolean handler in BitTrackerStage, simplifying the registration and handling logic for these IR instructions.
- IsSame in BitValue now checks Is32Bit for stricter equality. - RecalculateCounters in UnitTestBisectorSystem always sums Results and Baseline, removing special case logic. - Reformatted UnitTestRunner.cs to use tabs per project style.
Add NewlyObservedTransforms to PlanResult and update bisector logic to record, replay, and persist transforms first seen in each iteration. Simplify and standardize iteration status output, comment out redundant logs, and improve status formatting for clarity.
Consolidate multiple OutputStatusBisector calls in PrintIterationHeader into a single formatted line, improving log readability and reducing output verbosity.
Refactored UnitTestBisectorSystem.cs and Builder.cs by removing commented-out and redundant debug print statements, including PrintDisabledTransforms. Moved OutputStatusBisector and OutputStatus methods for clarity. The "Order" field is now always displayed, and RecalculateCounters logic is simplified. No changes to core functionality.
Replaces all "Print" status methods with unified "Output" methods for consistency. Adds FailureCount to BisectorState and updates all failure tracking logic to use it. Enhances OutputIterationStatus to show cumulative stats. Updates all bisector workflows to use new output methods and ensures accurate failure reporting.
Refactored UnitTestBisectorSystem to group related methods into logical regions (Entry Point, Plan Execution, Iteration Execution, State Management, Plan & Transform Helpers, File I/O, Output). Extracted and restructured helper methods for state management, plan parsing, output, and transform set building. Separated deterministic and random plan execution logic. Improved code clarity and maintainability without changing functional behavior.
Add hasRestartsExceeded to detect when unit test system restarts are exceeded. Update all plan execution paths to halt and record failure if this occurs. Enhance failure review file to indicate both compilation failures and restarts-exceeded conditions for improved diagnostics.
Relocated BitTrackerOperations from Mosa.Compiler.Framework.Stages to Mosa.Compiler.Framework.Analysis without changing its logic. Updated all references and using directives in affected files to use the new namespace.
Add notifications and disable checks for block transforms via CompilerHooks in BaseTransformStage. Make application launch status output conditional on MosaSettings.Diagnostic in BaseLauncher.
Allow OptimizationStage to opt-in to transform hook notifications and filtering by introducing an EnableTransformHooks property. Only stages with this property enabled will invoke transform hook logic, reducing unnecessary calls. Also, override the Name property in platform-specific OptimizationStages to include the platform prefix.
Added global.json to enforce usage of .NET SDK 10.0.203 with rollForward set to "latestFeature", ensuring consistent build environments across development and CI.
Added a private pipelinePool field to the Compiler class to track the PipelinePool instance during parallel compilation. Updated ExecuteCompile to assign and clear pipelinePool appropriately. Modified Stop to notify the PipelinePool when stopping the compiler. Also removed the global.json file specifying the .NET SDK version.
Improved code formatting and indentation across several files for readability. Removed unused using directives in BitTrackerStage.cs and RemoveUnreachableBlocks.cs. Added NotifyStop method to PipelinePool.cs to signal dispatcher shutdown. No functional changes except for the new NotifyStop method.
- Clarified default for `-bisect-stage` in command-line docs. - Refactored method compiler pipeline: always add `DeadBlockStage` with SSA, moved `SafePointStage` after `JumpOptimizationStage`, and removed `PlatformEdgeSplitStage`. - Added `EmitBinary` property to `ProtectedRegionLayoutStage` and updated logic to respect it. - Removed obsolete comment in `CodeGenerationStage.cs`. - Improved UI usability: enabled `CheckOnClick` for relevant checkboxes and cleared default text in `tbTransformStage`.
- Improve `ProcessReductionResult` in `Bisector.cs` to include items activated after experiment start in reduction candidates. - Add `Optimizations_Inline_Maximum`, `Optimizations_ScanWindow`, and `Optimizations_ReduceCodeSize` arguments to `-oSize` and `-oFast` profiles in `CommandLineArguments.cs`. - Reformat `UnitTestSerializer.cs` for style consistency; logic unchanged.
Comment on lines
+238
to
+241
| try | ||
| { | ||
| linkerMethodInfo = UnitTests.Linker.GetMethodInfo(TypeSystem, Linker, unitTestInfo); | ||
| } |
Comment on lines
+302
to
+310
| private Process StartTarget(string targetPath, string targetArguments, string workingDirectory) | ||
| { | ||
| var startInfo = new ProcessStartInfo | ||
| { | ||
| FileName = targetPath, | ||
| Arguments = targetArguments, | ||
| WorkingDirectory = workingDirectory, | ||
| UseShellExecute = false, | ||
| }; |
Comment on lines
+40
to
+41
| The ``-bisect-stage`` option specifies which compiler stage to bisect. The most common stage — and the default — is ``OptimizationStage``. You can override it with any ``BaseTransformStage`` subclass name (short or fully qualified). | ||
|
|
Comment on lines
696
to
703
| private void NotifyTransformObserved(string stageName, string transformName, string methodFullName) | ||
| { | ||
| if (!string.Equals(stageName, selectedStageName, StringComparison.Ordinal)) | ||
| if (!string.IsNullOrEmpty(mosaSettings.BisectorStage) && !string.Equals(stageName, mosaSettings.BisectorStage, StringComparison.Ordinal)) | ||
| return; | ||
|
|
||
| if (bisector == null && unitTestFilter != null && !methodFullName.Contains(unitTestFilter, StringComparison.Ordinal)) | ||
| return; | ||
|
|
Comment on lines
+161
to
+171
| [MosaUnitTest(Series = "U4")] | ||
| public static bool OptimizationTest22b(uint a) | ||
| { | ||
| return a % 2 == 1; | ||
| } | ||
|
|
||
| [MosaUnitTest(Series = "U4")] | ||
| public static bool OptimizationTest22c(uint a) | ||
| { | ||
| return a % 2 == 2; | ||
| } |
Comment on lines
+310
to
+313
| | `IsSubSignedOverflow(int a, int b)` | **`a + b` signed overflow** — note: despite the name, this checks signed overflow for the equivalent addition `a + b`, consistent with how x86/x64 OF is computed for SUB. Use it for `SubOverflowOut` instructions. | | ||
| | `IsSubSignedOverflow(long a, long b)` | Same, 64-bit | | ||
|
|
||
| > **Important gotcha**: `IsSubSignedOverflow(a, b)` does **not** test whether `a - b` overflows in the intuitive sense. It tests whether `a + b` overflows signed, which matches the hardware overflow flag for subtraction. So `IsSubSignedOverflow(int.MinValue, -1)` returns `true` (int.MinValue + (-1) underflows), while `IsSubSignedOverflow(int.MaxValue, -1)` returns `false`. |
Comment on lines
201
to
+205
| new Argument { Name = "-bisect-pairwise", Setting = Name.UnitTest_Bisector_Pairwise, Value = "true"}, | ||
| new Argument { Name = "-bisect-pairwise-off", Setting = Name.UnitTest_Bisector_Pairwise, Value = "false"}, | ||
| new Argument { Name = "-bisect-disabled-file", Setting = Name.UnitTest_Bisector_DisabledTransformsFile}, | ||
| new Argument { Name = "-bisect-state", Setting = Name.UnitTest_Bisector_StateFile}, | ||
| new Argument { Name = "-bisect-plan", Setting = Name.UnitTest_Bisector_Plan}, |
Removed all dynamic item observation logic from the Bisector, including ObserveItem and related suspect/candidate set management. Refactored transform tracking to use a single registered Transforms list, eliminating observed transform counts and simplifying state management. Updated all bisector logic, reporting, and compiler hooks to use the new RegisterTransform callback. Deleted all Fuzz0001–Fuzz0018 test files and their contents, removing hundreds of static fuzz test methods. Cleaned up output formatting, removed obsolete code, and improved error handling in the process supervisor.
Renamed the EnableTransformHooks property to AllowTransformHooks in BaseTransformStage and all derived OptimizationStage classes (ARM32, ARM64, x86, x64, and generic). Updated all references accordingly for naming consistency. No functional changes were made.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refactor bisector system, add supervisor, add unit tests, and fix transformations.
Major refactor of UnitTestBisectorSystem: introduces new bisector plans (DisableOne, EnableOne, RandomCombo, FailureInducing, Masking), robust state management, improved transform observation, and enhanced reporting. Adds Mosa.Utility.UnitTestBisector.Supervisor for process supervision and restart. Updates command-line options, documentation, and project files. Improves diagnostics, output, and testability throughout the bisector and unit test infrastructure.