Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 67 additions & 13 deletions .github/workflows/ci-benchmark.yml
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,29 @@ jobs:
- name: Setup CMake/Ninja
uses: lukka/get-cmake@latest

# Build latest Slang.
# - name: Build latest Slang
# run: |
# git clone --recursive https://github.com/shader-slang/slang.git
# cd slang
# mkdir build
# cmake -B build --preset default
# cmake --build build --config ${{ matrix.config }} --parallel
# DNI (do-not-integrate): build Slang from the descriptor-array by-reference fix.
# Composition under test (all three, matching real post-merge master):
# - shader-slang/slang#11939 dynamic-index local-copy legalization (the floor)
# - shader-slang/slang#11941 descriptor-table uniforms by reference (merged into #11939)
# - shader-slang/slang#11940 ParameterBlock-of-array emit-crash fix (independent)
# #11941 was merged into the floor branch (cuda-param-dynamic-index-floor), so that branch
# now carries BOTH #11939 and #11941; we clone it and fetch+merge #11940 so the implicit
# ParameterBlock-of-array this fix produces cannot hit the emit crash.
# NO slang-rhi bump here — unlike the retired F path (#779), the by-reference fix binds
# through the already-supported entry-point parameter-block sub-object path with zero
# host changes; SlangPy keeps its own external/slang-rhi pin (host-safety PR #1045).
- name: Build Slang (by-reference fix shader-slang/slang#11939+#11941 + #11940 — DNI)
run: |
git clone --recursive -b haaggarwal/cuda-param-dynamic-index-floor https://github.com/shader-slang/slang.git
cd slang
git config user.email "ci@slangpy.local"
git config user.name "slangpy-ci"
git fetch origin haaggarwal/cuda-pb-of-array-emit-fix
git merge --no-edit FETCH_HEAD
git submodule update --init --recursive
mkdir build
cmake -B build --preset default
cmake --build build --config ${{ matrix.config }} --parallel
Comment on lines +74 to +96

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Build relies on mutable personal branches with no pinned commit.

Cloning haaggarwal/cuda-param-dynamic-index-floor and merging haaggarwal/cuda-pb-of-array-emit-fix by branch name (not commit SHA) means results aren't reproducible if either branch is force-pushed between runs, and git merge --no-edit FETCH_HEAD has no conflict handling — a conflict would abort the build with a raw git error. Given the explicit DNI/throwaway nature of this branch, this is likely an acceptable tradeoff for one-off data collection.

🧰 Tools
🪛 zizmor (1.26.1)

[warning] 96-96: code injection via template expansion (template-injection): may expand into attacker-controllable code

(template-injection)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🔵 Trivial | 💤 Low value

Static analysis flags template expansion of matrix.config as potential injection.

zizmor flags both ${{ matrix.config }} usages, but config is a fixed literal ([Release]) defined in this same workflow's matrix, not attacker-controllable input — unlike the github.run_id case fixed in the previous review round. Likely a false positive, but for consistency with the env-var pattern already adopted for RUN_ID, consider routing matrix.config through an env var too to keep the lint clean.

Also applies to: 122-122

🧰 Tools
🪛 zizmor (1.26.1)

[warning] 96-96: code injection via template expansion (template-injection): may expand into attacker-controllable code

(template-injection)

Source: Linters/SAST tools


# Setup.
- name: Setup
Expand All @@ -100,19 +115,58 @@ jobs:
key: vcpkg-cache-${{ runner.os }}-${{ matrix.platform }}-${{ matrix.compiler }}-${{ hashFiles('vcpkg.json', 'external/vcpkg-triplets/**') }}

# Configure.
# DNI (do-not-integrate): point SlangPy at the locally-built by-reference-fix Slang so the
# benchmark exercises the compiler fix. SGL_LOCAL_SLANG swaps only the compiler; SlangPy
# keeps its own external/slang-rhi (SGL_LOCAL_RHI stays OFF).
- name: Configure
# run: python tools/ci.py --cmake-args="-DSGL_LOCAL_SLANG=ON -DSGL_LOCAL_SLANG_DIR=slang -DSGL_LOCAL_SLANG_BUILD_DIR=build/${{ matrix.config }}" configure
run: python tools/ci.py configure
run: python tools/ci.py --cmake-args="-DSGL_LOCAL_SLANG=ON -DSGL_LOCAL_SLANG_DIR=slang -DSGL_LOCAL_SLANG_BUILD_DIR=build/${{ matrix.config }}" configure

# Build.
- name: Build
run: python tools/ci.py build

# Benchmark (Python)
# Benchmark (Python) — DNI: run pytest directly with --benchmark-save instead of the
# Mongo upload path. The nvr-ci Mongo host is not always resolvable from these runners
# (getaddrinfo failures), and the Mongo upload exception aborts pytest before the
# terminal-summary table prints — so a Mongo outage silently loses all numbers. Saving
# to a local JSON makes the run self-contained: the "Name / Min / Max / Mean / Median /
# Stddev" table always prints to the log, and the JSON is uploaded as an artifact.
# Windows self-hosted runners have no bash — use pwsh. Each pytest is wrapped so a single
# device's failure never aborts the step before the others run; the always()-gated unlock
# step below is the real safety net that guarantees the shared runner's clocks are released.
- name: Benchmark (Python, Windows, GPU Clock Locked)
if: contains(matrix.flags, 'benchmark') && runner.os == 'Windows'
run: python tools/ci.py benchmark-python --run-id "${{ github.run_id }}" --mongodb-connection-string "${{ secrets.BENCHMARK_MONGODB_CONNECTION_STRING }}" --mongodb-database-name "nvr-ci" --lock-gpu-clocks
env:
RUN_ID: ${{ github.run_id }}
run: |
python tools/gpu_clock.py lock --ratio 0.7
python -m pytest slangpy/benchmarks -ra --device-types cuda --benchmark-save "dni-$env:RUN_ID-cuda"; $global:LASTEXITCODE = 0
python -m pytest slangpy/benchmarks -ra --device-types vulkan --benchmark-save "dni-$env:RUN_ID-vulkan"; $global:LASTEXITCODE = 0
python -m pytest slangpy/benchmarks -ra --device-types d3d12 --benchmark-save "dni-$env:RUN_ID-d3d12"; $global:LASTEXITCODE = 0
shell: pwsh

# Safety net: always release the GPU clocks on the shared self-hosted runner, even if the
# benchmark step above aborted — otherwise the clocks stay pinned at ratio 0.7 and corrupt
# every subsequent job on that runner.
- name: Unlock GPU clocks (Windows)
if: contains(matrix.flags, 'benchmark') && runner.os == 'Windows' && always()
run: python tools/gpu_clock.py unlock
shell: pwsh

- name: Benchmark (Python, Linux, GPU Clock Unlocked)
if: contains(matrix.flags, 'benchmark') && runner.os == 'Linux'
run: python tools/ci.py benchmark-python --run-id "${{ github.run_id }}" --mongodb-connection-string "${{ secrets.BENCHMARK_MONGODB_CONNECTION_STRING }}" --mongodb-database-name "nvr-ci"
env:
RUN_ID: ${{ github.run_id }}
run: |
python -m pytest slangpy/benchmarks -ra --device-types cuda --benchmark-save "dni-${RUN_ID}-cuda" || true
python -m pytest slangpy/benchmarks -ra --device-types vulkan --benchmark-save "dni-${RUN_ID}-vulkan" || true
shell: bash
Comment on lines 137 to +163

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Benchmark step failures are fully swallowed — no CI signal if all devices fail.

Every pytest invocation resets the exit code unconditionally ($global:LASTEXITCODE = 0 on Windows, || true on Linux), regardless of whether it succeeded or failed. This is more than "don't abort mid-step" — it means the step (and thus the job) will report success even if every single device benchmark crashes, with no JSON artifacts produced. Given the entire point of this DNI branch is to reliably collect RTX 5090 numbers, a silent all-device failure could go unnoticed unless someone reads the raw log closely.

Consider capturing/reporting per-device exit status (e.g., emit a ::warning:: annotation, or fail the step only if zero device types produced output) so a total failure is visible in the job status rather than only in the log text.

Example: surface failures instead of fully swallowing them
       - name: Benchmark (Python, Windows, GPU Clock Locked)
         if: contains(matrix.flags, 'benchmark') && runner.os == 'Windows'
         env:
           RUN_ID: ${{ github.run_id }}
         run: |
           python tools/gpu_clock.py lock --ratio 0.7
-          python -m pytest slangpy/benchmarks -ra --device-types cuda --benchmark-save "dni-$env:RUN_ID-cuda"; $global:LASTEXITCODE = 0
-          python -m pytest slangpy/benchmarks -ra --device-types vulkan --benchmark-save "dni-$env:RUN_ID-vulkan"; $global:LASTEXITCODE = 0
-          python -m pytest slangpy/benchmarks -ra --device-types d3d12 --benchmark-save "dni-$env:RUN_ID-d3d12"; $global:LASTEXITCODE = 0
+          python -m pytest slangpy/benchmarks -ra --device-types cuda --benchmark-save "dni-$env:RUN_ID-cuda"
+          if ($LASTEXITCODE -ne 0) { Write-Host "::warning::cuda benchmark failed" }
+          $global:LASTEXITCODE = 0
+          python -m pytest slangpy/benchmarks -ra --device-types vulkan --benchmark-save "dni-$env:RUN_ID-vulkan"
+          if ($LASTEXITCODE -ne 0) { Write-Host "::warning::vulkan benchmark failed" }
+          $global:LASTEXITCODE = 0
+          python -m pytest slangpy/benchmarks -ra --device-types d3d12 --benchmark-save "dni-$env:RUN_ID-d3d12"
+          if ($LASTEXITCODE -ne 0) { Write-Host "::warning::d3d12 benchmark failed" }
+          $global:LASTEXITCODE = 0
         shell: pwsh
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Benchmark (Python, Windows, GPU Clock Locked)
if: contains(matrix.flags, 'benchmark') && runner.os == 'Windows'
run: python tools/ci.py benchmark-python --run-id "${{ github.run_id }}" --mongodb-connection-string "${{ secrets.BENCHMARK_MONGODB_CONNECTION_STRING }}" --mongodb-database-name "nvr-ci" --lock-gpu-clocks
env:
RUN_ID: ${{ github.run_id }}
run: |
python tools/gpu_clock.py lock --ratio 0.7
python -m pytest slangpy/benchmarks -ra --device-types cuda --benchmark-save "dni-$env:RUN_ID-cuda"; $global:LASTEXITCODE = 0
python -m pytest slangpy/benchmarks -ra --device-types vulkan --benchmark-save "dni-$env:RUN_ID-vulkan"; $global:LASTEXITCODE = 0
python -m pytest slangpy/benchmarks -ra --device-types d3d12 --benchmark-save "dni-$env:RUN_ID-d3d12"; $global:LASTEXITCODE = 0
shell: pwsh
# Safety net: always release the GPU clocks on the shared self-hosted runner, even if the
# benchmark step above aborted — otherwise the clocks stay pinned at ratio 0.7 and corrupt
# every subsequent job on that runner.
- name: Unlock GPU clocks (Windows)
if: contains(matrix.flags, 'benchmark') && runner.os == 'Windows' && always()
run: python tools/gpu_clock.py unlock
shell: pwsh
- name: Benchmark (Python, Linux, GPU Clock Unlocked)
if: contains(matrix.flags, 'benchmark') && runner.os == 'Linux'
run: python tools/ci.py benchmark-python --run-id "${{ github.run_id }}" --mongodb-connection-string "${{ secrets.BENCHMARK_MONGODB_CONNECTION_STRING }}" --mongodb-database-name "nvr-ci"
env:
RUN_ID: ${{ github.run_id }}
run: |
python -m pytest slangpy/benchmarks -ra --device-types cuda --benchmark-save "dni-${RUN_ID}-cuda" || true
python -m pytest slangpy/benchmarks -ra --device-types vulkan --benchmark-save "dni-${RUN_ID}-vulkan" || true
shell: bash
- name: Benchmark (Python, Windows, GPU Clock Locked)
if: contains(matrix.flags, 'benchmark') && runner.os == 'Windows'
env:
RUN_ID: ${{ github.run_id }}
run: |
python tools/gpu_clock.py lock --ratio 0.7
python -m pytest slangpy/benchmarks -ra --device-types cuda --benchmark-save "dni-$env:RUN_ID-cuda"
if ($LASTEXITCODE -ne 0) { Write-Host "::warning::cuda benchmark failed" }
$global:LASTEXITCODE = 0
python -m pytest slangpy/benchmarks -ra --device-types vulkan --benchmark-save "dni-$env:RUN_ID-vulkan"
if ($LASTEXITCODE -ne 0) { Write-Host "::warning::vulkan benchmark failed" }
$global:LASTEXITCODE = 0
python -m pytest slangpy/benchmarks -ra --device-types d3d12 --benchmark-save "dni-$env:RUN_ID-d3d12"
if ($LASTEXITCODE -ne 0) { Write-Host "::warning::d3d12 benchmark failed" }
$global:LASTEXITCODE = 0
shell: pwsh
# Safety net: always release the GPU clocks on the shared self-hosted runner, even if the
# benchmark step above aborted — otherwise the clocks stay pinned at ratio 0.7 and corrupt
# every subsequent job on that runner.
- name: Unlock GPU clocks (Windows)
if: contains(matrix.flags, 'benchmark') && runner.os == 'Windows' && always()
run: python tools/gpu_clock.py unlock
shell: pwsh
- name: Benchmark (Python, Linux, GPU Clock Unlocked)
if: contains(matrix.flags, 'benchmark') && runner.os == 'Linux'
env:
RUN_ID: ${{ github.run_id }}
run: |
python -m pytest slangpy/benchmarks -ra --device-types cuda --benchmark-save "dni-${RUN_ID}-cuda" || true
python -m pytest slangpy/benchmarks -ra --device-types vulkan --benchmark-save "dni-${RUN_ID}-vulkan" || true
shell: bash


# Upload the saved benchmark JSON(s) so the medians survive even if the log is trimmed.
- name: Upload benchmark reports
if: contains(matrix.flags, 'benchmark') && always()
uses: actions/upload-artifact@v4
with:
name: benchmark-reports-${{ matrix.os }}
path: .benchmarks/**/*.json
if-no-files-found: warn
Comment on lines +165 to +172

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Unpinned action reference flagged by static analysis.

actions/upload-artifact@v4 isn't pinned to a commit hash, which zizmor reports as a required-policy error.

Pin to a commit SHA
-        uses: actions/upload-artifact@v4
+        uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa9 # v4.6.2
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Upload the saved benchmark JSON(s) so the medians survive even if the log is trimmed.
- name: Upload benchmark reports
if: contains(matrix.flags, 'benchmark') && always()
uses: actions/upload-artifact@v4
with:
name: benchmark-reports-${{ matrix.os }}
path: .benchmarks/**/*.json
if-no-files-found: warn
# Upload the saved benchmark JSON(s) so the medians survive even if the log is trimmed.
- name: Upload benchmark reports
if: contains(matrix.flags, 'benchmark') && always()
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa9 # v4.6.2
with:
name: benchmark-reports-${{ matrix.os }}
path: .benchmarks/**/*.json
if-no-files-found: warn
🧰 Tools
🪛 zizmor (1.26.1)

[error] 168-168: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

Source: Linters/SAST tools

112 changes: 112 additions & 0 deletions slangpy/benchmarks/test_benchmark_param_array.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

# Benchmarks the cost model of Slang's CUDA dynamic-index legalization
# (shader-slang/slang#11939), which rewrites a runtime index into a by-value
# kernel parameter array to go through an eager, whole-aggregate per-thread
# local copy made in the kernel prologue.
#
# The reviewer question this answers: the copy is paid up front for the whole
# array, per thread, per dispatch - it is NOT made on demand - so where is the
# crossover between the copy cost and the serial `.param` load chain it
# replaces, and does the copy itself become a regression for large parameters
# with few accesses?
#
# Three kernels x a parameter-size sweep:
# - pick_one: 1 dynamic access per thread (worst case: pay N, use 1)
# - sum_dynamic: N dynamic accesses per thread (best case: copy amortized)
# - sum_static: statically indexed control (legalization must not fire)
#
# Run the same file against a pre-#11939 Slang build to get the serial-chain
# baseline, and against a #11939 build for the local-copy numbers; the
# sum_static control should be identical on both, and any pick_one regression
# on the new build quantifies the eager-copy tax. Sizes stay <= 512 floats
# (2 KB) so the argument blob stays under CUDA's 4 KB entry-point-argument
# threshold and the functional API keeps the by-value fast path being measured.

import pytest
import numpy as np

import slangpy as spy
from slangpy.testing import helpers
from slangpy.testing.benchmark import BenchmarkSlangFunction

PARAM_SIZES = [16, 64, 256, 512]
CALL_SHAPE = (1024, 1024)


def _make_inputs(device: spy.Device, size: int):
weights = np.random.rand(size).astype(np.float32)
indices = np.random.randint(0, size, size=CALL_SHAPE).astype(np.uint32)
Comment on lines +38 to +39

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

No fixed RNG seed — non-reproducible failures.

np.random.rand/np.random.randint are unseeded, so a flaky/failing assertion can't be reproduced from the same test invocation. Consider a fixed seed for debuggability, since reproducibility doesn't affect the perf numbers being measured.

indices_tensor = spy.Tensor.from_numpy(device, indices)
result_tensor = spy.Tensor.empty(device, shape=CALL_SHAPE, dtype=float)
return weights, indices, indices_tensor, result_tensor
Comment on lines +37 to +42

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Missing return type annotation on _make_inputs.

Flagged by Ruff (ANN202). Low priority since it's a private test helper, not exported API surface.

Add return type
-def _make_inputs(device: spy.Device, size: int):
+def _make_inputs(
+    device: spy.Device, size: int
+) -> tuple[np.ndarray, np.ndarray, spy.Tensor, spy.Tensor]:
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def _make_inputs(device: spy.Device, size: int):
weights = np.random.rand(size).astype(np.float32)
indices = np.random.randint(0, size, size=CALL_SHAPE).astype(np.uint32)
indices_tensor = spy.Tensor.from_numpy(device, indices)
result_tensor = spy.Tensor.empty(device, shape=CALL_SHAPE, dtype=float)
return weights, indices, indices_tensor, result_tensor
def _make_inputs(
device: spy.Device, size: int
) -> tuple[np.ndarray, np.ndarray, spy.Tensor, spy.Tensor]:
weights = np.random.rand(size).astype(np.float32)
indices = np.random.randint(0, size, size=CALL_SHAPE).astype(np.uint32)
indices_tensor = spy.Tensor.from_numpy(device, indices)
result_tensor = spy.Tensor.empty(device, shape=CALL_SHAPE, dtype=float)
return weights, indices, indices_tensor, result_tensor
🧰 Tools
🪛 Ruff (0.15.20)

[warning] 37-37: Missing return type annotation for private function _make_inputs

(ANN202)

Source: Linters/SAST tools



@pytest.mark.parametrize("device_type", helpers.DEFAULT_DEVICE_TYPES)
@pytest.mark.parametrize("size", PARAM_SIZES)
def test_param_array_pick_one(
device_type: spy.DeviceType, size: int, benchmark_slang_function: BenchmarkSlangFunction
):
device = helpers.get_device(device_type)
weights, indices, indices_tensor, result_tensor = _make_inputs(device, size)

module = spy.Module(device.load_module("test_benchmark_param_array.slang"))
func = module.require_function(f"pick_one<{size}>")

benchmark_slang_function(
device,
func,
tid=spy.call_id(),
weights=weights.tolist(),
indices=indices_tensor,
_result=result_tensor,
)
assert np.allclose(result_tensor.to_numpy(), weights[indices])


@pytest.mark.parametrize("device_type", helpers.DEFAULT_DEVICE_TYPES)
@pytest.mark.parametrize("size", PARAM_SIZES)
def test_param_array_sum_dynamic(
device_type: spy.DeviceType, size: int, benchmark_slang_function: BenchmarkSlangFunction
):
device = helpers.get_device(device_type)
weights, _, indices_tensor, result_tensor = _make_inputs(device, size)

module = spy.Module(device.load_module("test_benchmark_param_array.slang"))
func = module.require_function(f"sum_dynamic<{size}>")

benchmark_slang_function(
device,
func,
tid=spy.call_id(),
weights=weights.tolist(),
indices=indices_tensor,
_result=result_tensor,
)
# Every thread sums all N elements (rotated by its start index).
assert np.allclose(result_tensor.to_numpy(), np.sum(weights), atol=1e-3)


@pytest.mark.parametrize("device_type", helpers.DEFAULT_DEVICE_TYPES)
@pytest.mark.parametrize("size", PARAM_SIZES)
def test_param_array_sum_static(
device_type: spy.DeviceType, size: int, benchmark_slang_function: BenchmarkSlangFunction
):
device = helpers.get_device(device_type)
weights, _, _, result_tensor = _make_inputs(device, size)

module = spy.Module(device.load_module("test_benchmark_param_array.slang"))
func = module.require_function(f"sum_static<{size}>")

benchmark_slang_function(
device,
func,
tid=spy.call_id(),
weights=weights.tolist(),
_result=result_tensor,
)
assert np.allclose(result_tensor.to_numpy(), np.sum(weights), atol=1e-3)


if __name__ == "__main__":
pytest.main([__file__, "-v", "-s"])
56 changes: 56 additions & 0 deletions slangpy/benchmarks/test_benchmark_param_array.slang
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

import slangpy;

// Benchmarks for the cost model of Slang's CUDA dynamic-index legalization
// (shader-slang/slang#11939): a runtime index into a by-value kernel parameter
// array is rewritten to go through an eager, whole-aggregate per-thread local
// copy made in the kernel prologue. These kernels sweep the two axes that
// bound that trade:
//
// - N: the parameter array size (the size of the eager copy)
// - accesses per thread: 1 (worst case for the copy: pay N, use 1)
// vs N (best case: copy amortized over many O(1) reads)
//
// plus a statically-indexed control where the legalization must not fire at
// all, isolating the copy cost from everything else.
//
// The array is plain data (float[N]) so that on CUDA it stays a by-value
// entry-point argument under both the old and new ABI (the by-reference
// conversion only applies to descriptor-bearing structs); the same script run
// against a pre-#11939 compiler therefore measures the serial `.param` chain
// this pass replaces.

// One dynamic access per thread: the eager copy of all N elements is paid to
// read a single element. This is the shape most likely to regress as N grows.
float pick_one<let N : int>(uint2 tid, float weights[N], Tensor<uint, 2> indices)
{
return weights[indices.load(tid)];
}

// N dynamic accesses per thread: the copy is amortized over N O(1) reads.
// This is the shape the legalization exists to accelerate.
float sum_dynamic<let N : int>(uint2 tid, float weights[N], Tensor<uint, 2> indices)
{
float result = 0.0;
uint base = indices.load(tid);
for (int i = 0; i < N; i++)
{
result += weights[(base + i) % N];
}
return result;
}

// Control: every index is a compile-time constant after unrolling, so the
// legalization must not fire and no local copy is made. Any time delta between
// this and sum_dynamic bounds the total cost of copy + dynamic addressing.
float sum_static<let N : int>(uint2 tid, float weights[N])
{
float result = 0.0;
[ForceUnroll]
for (int i = 0; i < N; i++)
{
result += weights[i];
}
return result;
}
10 changes: 10 additions & 0 deletions src/slangpy_ext/utils/slangpy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,16 @@ void NativeBoundVariableRuntime::write_shader_cursor_pre_dispatch(
// We have children, so generate call data for each child and
// store in a dictionary, then store the dictionary as the call data.
ShaderCursor child_field = cursor[m_variable_name.c_str()];
// A reference-typed field is a ConstantBuffer/ParameterBlock sub-object -
// e.g. Slang's CUDA target passes an entry-point uniform struct containing a
// fixed-size descriptor array by reference as an implicit ParameterBlock.
// Dereference before recursing so that children see a cursor whose
// shader_object() owns the offsets they extract: field lookups on a
// reference cursor auto-dereference (yielding sub-object-relative offsets),
// so a child that cached those offsets but wrote through the parent's
// shader object would silently corrupt memory.
if (child_field.is_reference())
child_field = child_field.dereference();
for (const auto& [name, child_ref] : *m_children) {
if (child_ref) {
nb::object child_value = value[name.c_str()];
Expand Down
16 changes: 16 additions & 0 deletions src/slangpy_ext/utils/slangpytensor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,22 @@ void TensorMarshall::ensure_binding_info_cached(ShaderCursor cursor, NativeBound
{
if (!m_cached_binding_info.primal.is_valid) {
ShaderCursor field = cursor[binding->variable_name()];
// The cached-offset fast path below assumes the tensor's fields live in
// `cursor.shader_object()` at offsets relative to that object. A
// reference-typed field (a ConstantBuffer/ParameterBlock sub-object) breaks
// that assumption: nested field lookups auto-dereference into the
// sub-object, so the cached offsets would be sub-object-relative while the
// write targets the parent object - silent corruption. Tensor types are
// never passed by reference themselves, and reference-typed *enclosing*
// structs are dereferenced before recursion (see
// NativeBoundVariableRuntime::write_shader_cursor_pre_dispatch), so fail
// loudly if one ever reaches this point.
SGL_CHECK(
!field.is_reference(),
"Tensor binding '{}' is reference-typed (a parameter-group sub-object); "
"the cached-offset writer does not support this shape",
binding->variable_name()
);
m_cached_binding_info = extract_binding_info(field);
}
}
Expand Down
9 changes: 9 additions & 0 deletions src/slangpy_ext/utils/slangpytorchtensor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,15 @@ void NativeTorchTensorMarshall::ensure_binding_info_cached(
{
if (!m_cached_binding_info.primal.is_valid) {
ShaderCursor field = cursor[binding->variable_name()];
// See TensorMarshall::ensure_binding_info_cached: the cached-offset writer
// requires the field's offsets to be relative to `cursor.shader_object()`,
// which a reference-typed (parameter-group sub-object) field violates.
SGL_CHECK(
!field.is_reference(),
"Torch tensor binding '{}' is reference-typed (a parameter-group sub-object); "
"the cached-offset writer does not support this shape",
binding->variable_name()
);
m_cached_binding_info = TensorMarshall::extract_binding_info(field);

// Determine copy-back flags from the Slang uniform type name.
Expand Down
25 changes: 22 additions & 3 deletions src/slangpy_ext/utils/slangpyvalue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,21 @@ void NativeValueMarshall::ensure_cached(ShaderCursor cursor, NativeBoundVariable
{
if (m_cached.is_valid)
return;
ShaderCursor field
= binding->direct_bind() ? cursor[binding->variable_name()] : cursor[binding->variable_name()]["value"];
ShaderCursor field = cursor[binding->variable_name()];
// A reference-typed field is a ConstantBuffer/ParameterBlock sub-object - e.g.
// Slang's CUDA target passes an entry-point uniform struct carrying a fixed-size
// descriptor array (such as the vectorized-array wrapper Array1DValueType holding
// tensors) by reference. Nested lookups below auto-dereference into the
// sub-object, making every cached offset sub-object-relative, so the write must
// target the sub-object's ShaderObject (see write_shader_cursor_pre_dispatch);
// writing through the parent object with these offsets would corrupt memory.
m_cached.field_is_reference = field.is_reference();
if (m_cached.field_is_reference) {
m_cached.field_index = cursor.find_field_index(binding->variable_name());
field = field.dereference();
}
if (!binding->direct_bind())
field = field["value"];
m_cached.value_offset = field.offset();
m_cached.value_type_layout = field.slang_type_layout();
m_cached.writer = get_shader_cursor_writer(m_cached.value_type_layout);
Expand All @@ -38,7 +51,13 @@ void NativeValueMarshall::write_shader_cursor_pre_dispatch(
AccessType primal_access = binding->access().first;
if (!value.is_none() && (primal_access == AccessType::read || primal_access == AccessType::readwrite)) {
ensure_cached(cursor, binding);
ShaderCursor value_cursor(cursor.shader_object(), m_cached.value_type_layout, m_cached.value_offset);
// For a reference-typed field the cached offsets are relative to the
// sub-object; re-resolve it (cheap: cached field index + object lookup) and
// write there instead of into the parent object.
ShaderObject* target_object = cursor.shader_object();
if (m_cached.field_is_reference)
target_object = cursor.get_field_by_index(m_cached.field_index).dereference().shader_object();
ShaderCursor value_cursor(target_object, m_cached.value_type_layout, m_cached.value_offset);
if (m_cached.writer) {
m_cached.writer(value_cursor, value);
} else {
Expand Down
6 changes: 6 additions & 0 deletions src/slangpy_ext/utils/slangpyvalue.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ class NativeValueMarshall : public NativeMarshall {
slang::TypeLayoutReflection* value_type_layout = nullptr; ///< Type layout for value field.
std::function<void(ShaderCursor&, nb::object)> writer; ///< Pre-resolved writer fn.
bool direct_bind{false}; ///< direct_bind value used when populating cache.
/// True when the bound field is reference-typed (a ConstantBuffer/ParameterBlock
/// sub-object, e.g. Slang's CUDA by-reference ABI for descriptor-table-carrying
/// uniforms). value_offset is then relative to the sub-object, and writes must
/// target the sub-object's ShaderObject rather than the parent's.
bool field_is_reference{false};
int32_t field_index{-1}; ///< Cached field index for the per-dispatch dereference.
bool is_valid = false;
};

Expand Down
Loading