Simplify vectorization guidelines and add a vectorization skill#131108
Merged
tannergooding merged 6 commits intoJul 21, 2026
Merged
Conversation
The general SIMD and hardware-intrinsics guidance now lives in the official docs (dotnet/docs#54834). Trim the repo doc to defer to it and keep only the dotnet/runtime-specific nuance: BoundedMemory AV testing, the LastIndexOf GC-hole case study, and the dotnet#64451 catalogue. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
SIMD guidance is a cross-cutting domain rather than a folder, so a task-triggered skill fits better than a file-scoped instructions file. The skill defers to the official docs and carries the repo-specific nuance (BoundedMemory AV testing, LoadUnsafe element-offset preference and the GC-hole rationale, acceleration-toggle env vars, benchmarking). Nudge toward it from copilot-instructions.md, matching the existing code-review/performance-benchmark pattern. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 1 pipeline(s). 15 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
A colon-space inside a plain YAML scalar is invalid, so the skill validator's frontmatter parse threw. Match the other skills and use a folded (>) block scalar. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates dotnet/runtime’s SIMD/vectorization guidance by shrinking the in-repo guidelines to runtime-specific nuance (and deferring general guidance to official Learn docs), and adds a new task-triggered vectorization skill to guide authoring/review of SIMD/hardware-intrinsics changes.
Changes:
- Replaces the large in-repo vectorization guideline document with a short “defer to official docs” version plus runtime-specific guidance (BoundedMemory AV testing, GC-hole case study, and links to real-world examples).
- Adds a new
.github/skills/vectorizationskill with SIMD authoring/testing/review checklists. - Updates
.github/copilot-instructions.mdto nudge using the newvectorizationskill for SIMD/hardware-intrinsics work.
Show a summary per file
| File | Description |
|---|---|
| docs/coding-guidelines/vectorization-guidelines.md | Trims guidance to runtime-specific nuance and points to official SIMD docs; includes BoundedMemory + GC-hole case study + examples link. |
| .github/skills/vectorization/SKILL.md | Introduces a new skill with SIMD authoring/testing/review rules and checklists, deferring to official docs and capturing runtime-specific pitfalls. |
| .github/copilot-instructions.md | Adds a one-line instruction to use the vectorization skill when working with SIMD/hardware intrinsics. |
Copilot's findings
Comments suppressed due to low confidence (3)
docs/coding-guidelines/vectorization-guidelines.md:36
BoundedMemory.Allocatealready populates the span with random data (per its remarks / implementation), so the extraRandom.Shared.NextBytes(...)is redundant and makes the sample look like initialization is required.
Random.Shared.NextBytes(MemoryMarshal.AsBytes(bounded.Span));
.github/skills/vectorization/SKILL.md:29
- "never cache them" is stronger than necessary and implies caching changes codegen. Since these are JIT-time constants, there's generally just no need to cache them for performance; a local
varwon't typically block constant folding.
2. **Start with `Vector128<T>`.** It's the common denominator accelerated on the broadest hardware, and
you don't need `Vector256`/`Vector512` for a correct, portable implementation. Add wider widths and
.github/skills/vectorization/SKILL.md:76
- Like the in-repo guideline doc, this text implies the failure mode is a thrown
AccessViolationException. TheBoundedMemorydocs describe it as causing an AV; depending on runtime / settings it may not be catchable.
suite with no overrides, with `DOTNET_EnableAVX2=0` (disables `Vector256`), and with
`DOTNET_EnableHWIntrinsic=0` (disables all intrinsics down to the software fallback). Build the
affected library and run its test project per the build/test workflow in
- Files reviewed: 3/3 changed files
- Comments generated: 2
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot's findings
Comments suppressed due to low confidence (2)
docs/coding-guidelines/vectorization-guidelines.md:22
- The
BoundedMemorydescription implies it always provides a poison (MEM_NOACCESS) page and therefore reliably AVs on out-of-bounds reads. ButBoundedMemoryfalls back to an unprotected allocator on some targets (e.g., Browser/WASI, and NETFRAMEWORK), where it won't reliably fault on OOB reads (see AllocateWithoutDataPopulationDefault / the Browser/WASI branch in AllocateWithoutDataPopulation). Please qualify this guidance so tests don’t over-rely on AV behavior on those platforms.
[`BoundedMemory`](/src/libraries/Common/tests/TestUtilities/System/Buffers/BoundedMemory.Creation.cs)
helper. It allocates a memory region immediately followed (or preceded) by a poison (`MEM_NOACCESS`)
page, so any out-of-bounds read throws an `AccessViolationException` during testing rather than silently
succeeding.
.github/skills/vectorization/SKILL.md:83
- This
BoundedMemorybullet reads as if a no-access poison page is always present. However,BoundedMemoryuses a fallback allocator on some targets (e.g., Browser/WASI, and NETFRAMEWORK) where page protection isn’t applied, so OOB reads won’t reliably throwAccessViolationException. Qualify the guidance accordingly.
- **Guard against out-of-bounds reads with `BoundedMemory`.**
[`BoundedMemory.Allocate<T>(count)`](/src/libraries/Common/tests/TestUtilities/System/Buffers/BoundedMemory.Creation.cs)
places a no-access page immediately after the buffer (use `PoisonPagePlacement.Before` for
backwards-iterating algorithms), so any read past the end throws `AccessViolationException` during
testing instead of silently succeeding. Always include lengths that aren't an exact multiple of the
- Files reviewed: 3/3 changed files
- Comments generated: 1
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot's findings
Comments suppressed due to low confidence (1)
.github/skills/vectorization/SKILL.md:28
- This overstates LINQ SIMD acceleration:
SumandAveragehaveVector<T>fast-paths whenTryGetSpansucceeds, butMax/Mindon't currently use SIMD in System.Linq. As written, this can mislead reviewers into assuming vectorization exists where it doesn't.
1. **Reach for the highest-level API that already does the job.** `Span<T>`/`string` methods,
`TensorPrimitives`, and the tensor types already vectorize many operations. LINQ is often vectorized
too — operators such as `Sum`, `Max`, `Min`, and `Average` accelerate when the source's underlying
span can be extracted. Don't hand-roll what's already optimized and tested.
- Files reviewed: 3/3 changed files
- Comments generated: 2
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
EgorBo
reviewed
Jul 20, 2026
eiriktsarpalis
approved these changes
Jul 21, 2026
tannergooding
deleted the
tannergooding-simplify-vectorization-guidelines
branch
July 21, 2026 13:39
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.
The general SIMD and hardware-intrinsics guidance now lives in the official docs, which were recently rewritten in dotnet/docs#54834. Our in-repo
docs/coding-guidelines/vectorization-guidelines.mdhad become largely duplicative and out of sync, so this trims it down to defer to the official article and keep only the dotnet/runtime-specific nuance:BoundedMemorytest helper for access-violation testing (with a repo-relative link and a short usage example).LastIndexOfGC-hole case study (Vectorize {Last}IndexOf{Any} and {Last}IndexOfAnyExcept without code duplication #73768 / fix Fixing SpanHelpers.LastIndexOfAnyValueType to no longer create out of bounds GC refs #75857) behind the "prefer the element-offset overloads" rule.The lane-crossing worked example that the official rewrite dropped is being restored upstream in dotnet/docs#54840 rather than kept here, since it''s general teaching content and belongs where it stays in sync.
Also adds a
vectorizationskill covering both authoring and reviewing SIMD code. Vectorization is a cross-cutting domain rather than a folder, so a task-triggered skill fits better than a file-scoped.instructions.md. The skill defers to the official docs and carries the same repo-specific nuance (prefer the span/LoadUnsafeelement-offset overloads,BoundedMemoryAV testing, the acceleration-toggle env vars,char/boolreinterpretation, and benchmarking before committing complexity), plus authoring/testing/review checklists.copilot-instructions.mdgets a one-line nudge toward it, matching the existingcode-review/performance-benchmarkpattern.Note
This PR description was drafted by GitHub Copilot.