diff --git a/.github/workflows/ci-benchmark.yml b/.github/workflows/ci-benchmark.yml index fa36a6c6b..4bdb3f5e8 100644 --- a/.github/workflows/ci-benchmark.yml +++ b/.github/workflows/ci-benchmark.yml @@ -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 # Setup. - name: Setup @@ -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 + + # 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 diff --git a/slangpy/benchmarks/test_benchmark_param_array.py b/slangpy/benchmarks/test_benchmark_param_array.py new file mode 100644 index 000000000..847c35d29 --- /dev/null +++ b/slangpy/benchmarks/test_benchmark_param_array.py @@ -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) + 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 + + +@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"]) diff --git a/slangpy/benchmarks/test_benchmark_param_array.slang b/slangpy/benchmarks/test_benchmark_param_array.slang new file mode 100644 index 000000000..665a79116 --- /dev/null +++ b/slangpy/benchmarks/test_benchmark_param_array.slang @@ -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(uint2 tid, float weights[N], Tensor 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(uint2 tid, float weights[N], Tensor 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(uint2 tid, float weights[N]) +{ + float result = 0.0; + [ForceUnroll] + for (int i = 0; i < N; i++) + { + result += weights[i]; + } + return result; +} diff --git a/src/slangpy_ext/utils/slangpy.cpp b/src/slangpy_ext/utils/slangpy.cpp index ca4be81d0..5527c256c 100644 --- a/src/slangpy_ext/utils/slangpy.cpp +++ b/src/slangpy_ext/utils/slangpy.cpp @@ -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()]; diff --git a/src/slangpy_ext/utils/slangpytensor.cpp b/src/slangpy_ext/utils/slangpytensor.cpp index a0d71c466..f56c71d09 100644 --- a/src/slangpy_ext/utils/slangpytensor.cpp +++ b/src/slangpy_ext/utils/slangpytensor.cpp @@ -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); } } diff --git a/src/slangpy_ext/utils/slangpytorchtensor.cpp b/src/slangpy_ext/utils/slangpytorchtensor.cpp index 8c4d4b56f..ae1a4f767 100644 --- a/src/slangpy_ext/utils/slangpytorchtensor.cpp +++ b/src/slangpy_ext/utils/slangpytorchtensor.cpp @@ -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. diff --git a/src/slangpy_ext/utils/slangpyvalue.cpp b/src/slangpy_ext/utils/slangpyvalue.cpp index 6f6543ff9..7f3acc32b 100644 --- a/src/slangpy_ext/utils/slangpyvalue.cpp +++ b/src/slangpy_ext/utils/slangpyvalue.cpp @@ -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); @@ -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 { diff --git a/src/slangpy_ext/utils/slangpyvalue.h b/src/slangpy_ext/utils/slangpyvalue.h index 41040407d..494963218 100644 --- a/src/slangpy_ext/utils/slangpyvalue.h +++ b/src/slangpy_ext/utils/slangpyvalue.h @@ -35,6 +35,12 @@ class NativeValueMarshall : public NativeMarshall { slang::TypeLayoutReflection* value_type_layout = nullptr; ///< Type layout for value field. std::function 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; };