Skip to content

Clarify WASM shuffle/swizzle codegen comments#130934

Merged
tannergooding merged 1 commit into
dotnet:mainfrom
tannergooding:tannergooding-wasm-simd-shuffle-followup
Jul 17, 2026
Merged

Clarify WASM shuffle/swizzle codegen comments#130934
tannergooding merged 1 commit into
dotnet:mainfrom
tannergooding:tannergooding-wasm-simd-shuffle-followup

Conversation

@tannergooding

Copy link
Copy Markdown
Member

Post-merge follow-up to #130850, addressing three review comments from @adamperlin.

  • hwintrinsiccodegenwasm.cpp: reword the contained-mask Swizzle->i8x16.shuffle remark. On Wasm genConsumeMultiOpOperands only updates liveness and doesn't touch the value stack, so the comment now says "prior codegen left the source on the value stack once" rather than attributing it to genConsumeMultiOpOperands.

  • gentree.cpp (TARGET_WASM Shuffle-lowering branch): add a lead-in comment describing the element->byte-index expansion -- each element-granular selector expands into elementSize consecutive byte indices for i8x16.swizzle, and an out-of-range selector becomes 0xFF bytes so swizzle's native index >= 16 -> 0 behavior zero-fills that element. Dropped the now-redundant inline else-block comment.

The third comment (lowerwasm.cpp, feeding a constant-zero second operand to i8x16.shuffle instead of the source twice) is addressed in the thread rather than in code: a local.get of the existing source is smaller than materializing a 16-byte v128.const zero, and it can't remove the special case since the Swizzle node is 2-operand (source, mask->immediate) with no second-vector slot.

Comment-only; no functional change.

Note

This PR description was drafted by Copilot on behalf of @tannergooding.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 17, 2026 00:04
@github-actions github-actions Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jul 17, 2026
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 5 pipeline(s).
10 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates explanatory comments in the WASM SIMD shuffle/swizzle code paths in the JIT, clarifying how masks map to i8x16.shuffle / i8x16.swizzle and removing redundant commentary.

Changes:

  • Reword the Swizzle-contained-mask comment in genHWIntrinsic (WASM codegen) to avoid attributing behavior to a specific helper.
  • Add a lead-in comment in gtNewSimdShuffleNode (WASM lowering branch) describing element-to-byte-index expansion for i8x16.swizzle, and remove a now-redundant inline comment.
Show a summary per file
File Description
src/coreclr/jit/hwintrinsiccodegenwasm.cpp Comment clarification for WASM Swizzle lowering to i8x16.shuffle.
src/coreclr/jit/gentree.cpp Adds/cleans up comments documenting WASM shuffle-to-swizzle mask expansion semantics.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 1

Comment thread src/coreclr/jit/hwintrinsiccodegenwasm.cpp
@adamperlin

Copy link
Copy Markdown
Contributor

/ba-g infra failures

@tannergooding
tannergooding merged commit e220bf0 into dotnet:main Jul 17, 2026
139 of 142 checks passed
@tannergooding
tannergooding deleted the tannergooding-wasm-simd-shuffle-followup branch July 17, 2026 17:39
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 11.0-preview7 milestone Jul 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants