Add general .NET vectorization skill - #1045
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Skill Coverage Report
Uncovered:
|
There was a problem hiding this comment.
Pull request overview
This PR retires the experimental exp-simd-vectorization skill/eval and introduces a new general-purpose .NET SIMD vectorization skill under the dotnet-advanced plugin, along with a new evaluation suite and fixtures to cover authoring and review scenarios.
Changes:
- Removed the
dotnet-experimental/exp-simd-vectorizationskill and its associated eval + fixtures. - Added
dotnet-advanced/vectorizationskill content with detailed guidance aligned to the official .NET SIMD guidance/templates. - Added a new
tests/dotnet-advanced/vectorizationeval with a set of targeted fixtures/projects, and updated CODEOWNERS + plugin README indexing.
Show a summary per file
| File | Description |
|---|---|
| tests/dotnet-experimental/exp-simd-vectorization/fixtures/simd-tensor-primitives-product.cs | Removed experimental fixture as part of retiring the eval. |
| tests/dotnet-experimental/exp-simd-vectorization/fixtures/simd-tensor-primitives-minmax.cs | Removed experimental fixture as part of retiring the eval. |
| tests/dotnet-experimental/exp-simd-vectorization/fixtures/simd-no-opportunity-catalog.cs | Removed experimental fixture as part of retiring the eval. |
| tests/dotnet-experimental/exp-simd-vectorization/fixtures/simd-bit-reverser.cs | Removed experimental fixture as part of retiring the eval. |
| tests/dotnet-experimental/exp-simd-vectorization/eval.yaml | Removed the experimental eval definition. |
| plugins/dotnet-experimental/skills/exp-simd-vectorization/SKILL.md | Removed the experimental SIMD skill content. |
| tests/dotnet-advanced/vectorization/fixtures/ZeroBytes.cs | Added a new fixture for vectorization review/edge-case detection. |
| tests/dotnet-advanced/vectorization/fixtures/TailSearch.cs | Added a new fixture for tail-handling correctness/memory-safety review. |
| tests/dotnet-advanced/vectorization/fixtures/LastMatch.cs | Added a new fixture for managed-reference safety review. |
| tests/dotnet-advanced/vectorization/fixtures/AsciiLetters.cs | Added a new fixture for portability/runtime-correctness review. |
| tests/dotnet-advanced/vectorization/fixtures/ContainsZero.cs | Added a new fixture for architecture-consistent behavior review. |
| tests/dotnet-advanced/vectorization/fixtures/sum-values/SumValues.csproj | Added runnable project fixture for a reduction repair task. |
| tests/dotnet-advanced/vectorization/fixtures/sum-values/SumValues.cs | Added reduction fixture source for a repair task. |
| tests/dotnet-advanced/vectorization/fixtures/sum-values/Program.cs | Added runnable validation harness for the reduction fixture. |
| tests/dotnet-advanced/vectorization/fixtures/product/Product.csproj | Added runnable project fixture for TensorPrimitives-based optimization. |
| tests/dotnet-advanced/vectorization/fixtures/product/Product.cs | Added product implementation fixture for optimization. |
| tests/dotnet-advanced/vectorization/fixtures/product/Program.cs | Added runnable validation harness for product fixture behavior. |
| tests/dotnet-advanced/vectorization/fixtures/conditional-increment/ConditionalIncrement.csproj | Added runnable project fixture for conditional increment authoring task. |
| tests/dotnet-advanced/vectorization/fixtures/conditional-increment/Program.cs | Added runnable validation harness for conditional increment behavior. |
| tests/dotnet-advanced/vectorization/fixtures/conditional-increment/ConditionalIncrement.cs | Updated fixture implementation shape (static class, formatting) for the new eval. |
| tests/dotnet-advanced/vectorization/fixtures/widen-clamp/WidenClamp.csproj | Added runnable project fixture for widening dispatch task. |
| tests/dotnet-advanced/vectorization/fixtures/widen-clamp/Program.cs | Added runnable validation harness for widening clamp behavior. |
| tests/dotnet-advanced/vectorization/fixtures/widen-clamp/ClampNegative.cs | Added baseline Vector128 implementation fixture for widening-to-Vector256 task. |
| tests/dotnet-advanced/vectorization/eval.yaml | Added the new vectorization evaluation with multiple targeted stimuli + graders. |
| plugins/dotnet-advanced/skills/vectorization/SKILL.md | Added the new general vectorization skill guidance under dotnet-advanced. |
| plugins/dotnet-advanced/README.md | Indexed the new vectorization skill in the plugin README. |
| .github/CODEOWNERS | Reassigned ownership from experimental paths to the new dotnet-advanced skill/test paths. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 27/27 changed files
- Comments generated: 0
- Review effort level: Lite
📊 Skill Evaluation Results2 model/skill results across 1 skill and 2 models — ✅ 2 improved, ➖ 0 not proven improved, Measurement identity: evaluated commit Measurement health: 2 expected / 2 observed / 2 written; 0 missing, 0 unexpected, 0 invalid; 0 recovered comparison error slots and 0 unresolved comparison error slots. Objective completion gate: not enabled. Aggregate completion transitions are telemetry only, so this report does not claim that zero objective regressions were proven. A result passes only when the aggregate net win across distinct-stimulus votes is at least 20% and an exact one-sided sign-test result of
ℹ️ How to read this report
✅ Improved — vectorization (claude-sonnet-4.6)Why: Net win +70.0% (8W/1T/1L over 10 stimulus vote(s), sign test p=0.020), mean preference +40.0% across 10 paired run(s) — credibly better Next action: Fix activation gaps; Review overfit evidence. State: Gate evidence: n=10; 8W/1T/1L; d=9; p=0.020; net +70.0% Warnings: Activation: isolated 5/9; plugin 4/9 Overfit: Moderate (score 0.43) Repeated-run reliability (not used by the gate): 10 paired runs (8W/1T/1L). Weak or warning scenarios:
Illustrative judge evidence:
This is one example, not the aggregate verdict. Open Full Results for every judgment. ✅ Improved — vectorization (gpt-5.6-luna)Why: Net win +80.0% (8W/2T/0L over 10 stimulus vote(s), sign test p=0.004), mean preference +44.0% across 10 paired run(s) — credibly better Next action: Fix activation gaps; Review overfit evidence. State: Gate evidence: n=10; 8W/2T/0L; d=8; p=0.004; net +80.0% Warnings: Activation: isolated 9/9; plugin 5/9 Overfit: Moderate (score 0.32) Repeated-run reliability (not used by the gate): 10 paired runs (8W/2T/0L). Weak or warning scenarios:
Illustrative judge evidence:
This is one example, not the aggregate verdict. Open Full Results for every judgment. 🔍 Full Results - all metrics and investigation details ▶ Sessions Visualisation -- interactive replay of all evaluation sessions |
|
✅ Evaluation passed for |
|
Thanks a bunch for the thorough skill and eval coverage here. I reviewed the skill, fixtures, and eval design across Claude, GPT, and Gemini. The SIMD guidance is strong overall. I found three areas worth adjusting:
After these skill and eval changes, a fresh official evaluation would give useful evidence for activation and behavior on the revised content. (Copilot, commenting on Abhitej's behalf.) |
This is an area where even if the edge is technically allowed, it remains an inherently unsafe and dangerous operation that leads to a large number of bugs and potential security issues, especially where lesser LLMs infer it to be extendable to other cases or make mistakes. Our official guidance, especially with the work around
The The "scalar epilogue" would be a failure of the skill to correctly vectorize the method (i.e. is invalid). Leaving scalar epilogues is essentially never required and is something that a skill that is explicitly meant to vectorize the code should not be leaving behind. I'll look at doing cleanup for other cases mentioned as appropriate. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Did some cleanup. The main thing worth pointing out is that the requirements for Even where these things are "technically equivalent", it is explicitly unnecessary risk and complexity that we want to push models away from for such an advanced skill. The simpler safe thing is the correct thing and what we want to push as the idiomatic way to handle it, how we want existing code fixed up to be as well to reduce the known risk and bugs that occur. |
📊 Skill Evaluation Results2 model/skill results across 1 skill and 2 models — ✅ 0 improved, ➖ 2 not proven improved, Measurement identity: evaluated commit Measurement health: 2 expected / 2 observed / 2 written; 0 missing, 0 unexpected, 0 invalid; 0 recovered comparison error slots and 0 unresolved comparison error slots. Objective completion gate: not enabled. Aggregate completion transitions are telemetry only, so this report does not claim that zero objective regressions were proven. A result passes only when the aggregate net win across distinct-stimulus votes is at least 20% and an exact one-sided sign-test result of
ℹ️ How to read this report
➖ Not proven improved — vectorization (claude-sonnet-4.6)Why: Net win -20.0% (3W/2T/5L over 10 stimulus vote(s), sign test p=0.363), mean preference -14.0% across 10 paired run(s) — no improvement Next action: Inspect tied or lost stimuli and fix inconsistent skill behavior. State: Gate evidence: n=10; 3W/2T/5L; d=8; p=0.363; net -20.0% Warnings: Activation: isolated 5/9; plugin 4/9 Overfit: Moderate (score 0.44) Repeated-run reliability (not used by the gate): 10 paired runs (3W/2T/5L). Weak or warning scenarios:
Illustrative judge evidence:
This is one example, not the aggregate verdict. Open Full Results for every judgment. ➖ Not proven improved — vectorization (gpt-5.6-luna)Why: Net win +50.0% (6W/3T/1L over 10 stimulus vote(s), sign test p=0.063), mean preference +26.0% across 10 paired run(s) — not credible (sign test p=0.063 > 0.05) Next action: Inspect tied or lost stimuli and fix inconsistent skill behavior. State: Gate evidence: n=10; 6W/3T/1L; d=7; p=0.063; net +50.0% Warnings: Activation: isolated 7/9; plugin 6/9 Overfit: Moderate (score 0.33) Repeated-run reliability (not used by the gate): 10 paired runs (6W/3T/1L). Weak or warning scenarios:
Illustrative judge evidence:
This is one example, not the aggregate verdict. Open Full Results for every judgment. 🔍 Full Results - all metrics and investigation details
▶ Sessions Visualisation -- interactive replay of all evaluation sessions |
AbhitejJohn
left a comment
There was a problem hiding this comment.
Thanks for the detailed responses and updates. Taking your comment as the authoritative design intent, I agree that the skill can prohibit one-past-end managed references as a conservative policy, even though the runtime permits them when they are not dereferenced.
I think the current text still differs from that intent. SKILL.md says a collection can leave the reference untracked, and the LastMatch rubric requires that explanation. Would you be open to stating this as an intentional safety policy - permitted by the runtime, but prohibited by this guidance because it is fragile and easy to misuse - without attributing it to lost GC tracking?
On the eval checks, I agree that Vector128.IsHardwareAccelerated, Vector128.Create(span), CopyTo(span), and the prohibition on unmeasured wider paths enforce requirements that the skill states explicitly. In particular, the disabled-intrinsics test proves equivalent results, but it cannot prove that the scalar branch ran because Vector128 operations have a correct software fallback. Keeping the Vector128.IsHardwareAccelerated check makes sense.
My remaining eval question is specifically about requiring the literal ConditionalSelect token. A tail can remain fully vectorized and avoid duplicate work by masking the overlapping vector with bitwise operations. Conceptually:
Vector128<int> tail = Vector128.Create(finalFullSpan);
Vector128<int> mask = CreateMaskForUnprocessedLanes(overlap);
sum += tail & mask;Here, lanes already processed by the main loop are zero in mask, and lanes that belong to the remainder are all bits set. This is equivalent for the reduction to ConditionalSelect(mask, tail, Vector128<int>.Zero). It has no scalar epilogue and does not count overlapping lanes twice.
Would it make sense for the rubric to accept either ConditionalSelect or an equivalent vector masking operation, while the existing nonmultiple-length execution cases verify that no lanes are duplicated?
I also read the latest official result as 0 improved, 2 not proven improved. That does not prove a correctness regression, but it also does not establish an improvement at this head.
(Copilot, commenting on Abhitej's behalf.)
|
✅ Approved by @AbhitejJohn. cc @dotnet/skills-merge-approvers — ready to merge. |
Have a fix for that up now.
The conceptual simplification can actually be worse for codegen, quality, and extensibility. We explicitly recognize and optimize the |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
📊 Skill Evaluation Results2 model/skill results across 1 skill and 2 models — ✅ 2 improved, ➖ 0 not proven improved, Measurement identity: evaluated commit Measurement health: 2 expected / 2 observed / 2 written; 0 missing, 0 unexpected, 0 invalid; 0 recovered comparison error slots and 0 unresolved comparison error slots. Objective completion gate: not enabled. Aggregate completion transitions are telemetry only, so this report does not claim that zero objective regressions were proven. A result passes only when the aggregate net win across distinct-stimulus votes is at least 20% and an exact one-sided sign-test result of
ℹ️ How to read this report
✅ Improved — vectorization (claude-sonnet-4.6)Why: Net win +60.0% (7W/2T/1L over 10 stimulus vote(s), sign test p=0.035), mean preference +42.0% across 10 paired run(s) — credibly better Next action: Fix activation gaps; Review overfit evidence. State: Gate evidence: n=10; 7W/2T/1L; d=8; p=0.035; net +60.0% Warnings: Activation: isolated 4/9; plugin 4/9 Overfit: Moderate (score 0.44) Repeated-run reliability (not used by the gate): 10 paired runs (7W/2T/1L). Weak or warning scenarios:
Illustrative judge evidence:
This is one example, not the aggregate verdict. Open Full Results for every judgment. ✅ Improved — vectorization (gpt-5.6-luna)Why: Net win +60.0% (6W/4T/0L over 10 stimulus vote(s), sign test p=0.016), mean preference +30.0% across 10 paired run(s) — credibly better Next action: Fix activation gaps; Review overfit evidence. State: Gate evidence: n=10; 6W/4T/0L; d=6; p=0.016; net +60.0% Warnings: Activation: isolated 8/9; plugin 5/9 Overfit: Moderate (score 0.34) Repeated-run reliability (not used by the gate): 10 paired runs (6W/4T/0L). Weak or warning scenarios:
Illustrative judge evidence:
This is one example, not the aggregate verdict. Open Full Results for every judgment. 🔍 Full Results - all metrics and investigation details ▶ Sessions Visualisation -- interactive replay of all evaluation sessions |
|
Thanks for the fixes tanner, LGTM. Tagging @artl93 for any additional thoughts. Otherwise, looks good to go. |
|
@AbhitejJohn - If your referring to expertise on vectorization, I cannot comment. I rely on Tanner. |
|
@artl93 : Sounds good, merging this in then :) Thanks for the addition Tanner, looking forward to more. |
Replaces #1044, recreated from a branch hosted directly in
dotnet/skillsso the full CI workflow can run.Ports dotnet/runtime's vectorization guidance into the general
dotnet-advancedplugin and retires the experimental SIMD skill.The port keeps portable API selection, safe span loads and stores, self-contained width dispatch, vectorized remainder handling, fallback validation, and measured wider-width decisions aligned with the official .NET guidance. It also adds ten stimuli covering review, authoring, framework reuse, portability, memory safety, and routing boundaries.
Targeted local model runs exposed several places where concise guidance was not enough:
LoadUnsafe/StoreUnsafewith a scalar epilogue. Adding the small safe inner-loop template changed the GPT-5.5 result toVector128.Create/CopyTowith a preloaded overlapping vector tail.Vector256 && Length >= Countfollowed byVector128 && Length >= Countcascade. The skill now explicitly defers to the official comprehensive templates and requires each widest-first hardware block to select its vector helper or dedicated small-input helper and return. Gemini and MAI reruns then produced that self-contained dispatch shape.SumValuesfixture from managed-reference loading to safe span creation with a masked vector tail. Gemini retainedMemoryMarshal.GetReference/LoadUnsafedespite marking its own result as passing, which motivated deterministic graders for safe loading and fallback guards rather than relying only on a judge rubric.These were targeted local stimulus runs rather than the official Vally comparison; the PR evaluation remains the source of truth for measured baseline delta.
Closes #894.
Note
This pull request description was drafted with GitHub Copilot.