Fix lowering of swizzled lvalues

[kmshanah]

• Set the size of the elementIndices instead of adding the size as a value
• Fix backpermute lambda to modify the passed in indicies (was working on a copy)

[jhelferty-nv]

Automated notice (PR board sync) — do not reply to this comment.

Auto-assigned @jhelferty-nv as shepherd for this Community PR.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 476e5f20-587c-4ffb-921c-a33c6779c085

📥 Commits

Reviewing files that changed from the base of the PR and between 75ecfe6 and dbcf890.

📒 Files selected for processing (1)

• source/slang/slang-lower-to-ir.cpp

Included review availability: Your plan provides up to 8 included reviews per hour; 3 remain after this review.

---

📝 Walkthrough

Walkthrough

Nested l-value swizzle lowering now preserves remapped element indices and correctly sizes the destination index list. A compute shader regression test validates nested vector swizzle writes across supported backends.

Changes

Nested swizzle lowering

Layer / File(s)	Summary
Fix nested swizzle index remapping source/slang/slang-lower-to-ir.cpp	backpermute now receives the destination array by reference. SwizzledLValueInfo::elementIndices is pre-sized with setCount(elementCount).
Add nested swizzle write regression test tests/language-feature/swizzles/vector-swizzle-write-swizzle.slang, tests/language-feature/swizzles/vector-swizzle-write-swizzle.slang.expected.txt	The compute shader writes through nested vector swizzles. The expected output records the resulting float values.

Suggested reviewers: jhelferty-nv, csyonghe, saipraveenb25

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: fixing the lowering of swizzled l-values.
Description check	✅ Passed	The description directly explains both implementation fixes in the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 too large.)

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[nv-slang-bot]

Thanks for the quick fix — the 2-line change matches the root cause of #12768 exactly: .add((uint32_t)elementCount) → .setCount(...) sizes the composed index list correctly (the old form produced a length-1 list holding the count 3), and auto bs → auto& bs makes backpermute write through the real ShortList instead of a discarded copy.

Two small follow-ups would let this PR fully close the loop on the issue:

1. Add Closes #12768 to the description — the description has no closing keyword, so the issue won't auto-close when this merges.
2. A regression test — the github-actions[bot] review above (the "No regression test for the fixed nested-swizzle l-value path" comment) already lays out a good GPU-free approach. A CPU COMPARE_COMPUTE that writes through a nested vector swizzle and reads the components back would lock in the exact wrong-component store this fixes; tests/language-feature/swizzles/matrix-swizzle-write-swizzle.slang is the matrix analogue to model it on.

_{🤖 Generated by an automated Slang coworker — may be inaccurate. A human maintainer should verify.}

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 80fd7193-7593-4e61-ab80-d1be82dfe5db

📥 Commits

Reviewing files that changed from the base of the PR and between 6abab25 and 75ecfe6.

📒 Files selected for processing (2)

• source/slang/slang-lower-to-ir.cpp
• tests/language-feature/swizzles/vector-swizzle-write-swizzle.slang

Included review availability: Your plan provides up to 8 included reviews per hour; 4 remain after this review.

[jkwak-work]

Looks good to me

[jkwak-work]

Fixes #12768

[jkwak-work]

PR looks good to me.

I retriggered the failed aarch64-windows tests.

[jkwak-work]

*.expected.txt is a less preferred way to check the result due to its lower readability.
And it looks like this file is redundant to what we already have:

// CHECK: 2.0
// CHECK-NEXT: 1.0
// CHECK-NEXT: 0.0
// CHECK-NEXT: 9.0

Please delete this file.

[github-actions]

Verdict: ✅ Clean — no significant issues found

Fixes two defects in visitSwizzleExpr when lowering a swizzle-of-swizzle lvalue write (e.g. a.zw.y = 9.0, a.xyzw.yx = float2(...)): (1) elementIndices.add((uint32_t)elementCount) appended the count as a value (leaving getCount() == 1) rather than sizing the list, and (2) the backpermute lambda took its output list by value, so the remapped indices were written to a discarded copy. Both are corrected (setCount + pass-by-reference), and a regression test is added.

Changes Overview

Nested vector swizzle lvalue lowering (source/slang/slang-lower-to-ir.cpp)

• In the SwizzledLValue collapse branch, swizzledLValue->elementIndices is now sized with setCount((uint32_t)elementCount) instead of add(...), and the backpermute lambda takes resultElements by reference (auto&) so the permuted indices land in the real list. The sourceElements/indices/resultElements renames make the gather explicit. This matches the existing setCount+fill-loop idiom already used on the assign path (slang-lower-to-ir.cpp:10174-10176). The SwizzledMatrixLValue branch was unaffected functionally (its MatrixCoord[4] C array decayed to a pointer under pass-by-value) but is now uniform.

Regression test (tests/language-feature/swizzles/vector-swizzle-write-swizzle.slang)

• New compute test (d3d/vk/mtl/cpu) exercising a single-element (a.zw.y) and a multi-element (a.xyzw.yx) outer swizzle write; the multi-element case is exactly what the old add(...) shape mishandled. Expected buffer [2,1,0,9].

_{reviewed: 93438cc · diff sha256 1c8f977d08b1}