From 20bd48659d0009de5477380c335e2419f4c66f8b Mon Sep 17 00:00:00 2001 From: venkataram-nv Date: Mon, 12 Aug 2024 14:18:02 -0700 Subject: Warn when inout parameter is never written (#4777) Addresses #4698 as one approach to diagnose the potential problem. Emit warnings when a user marks a parameter as `inout` but never writes to it in the function. A new intrinsic function `unmodified(out T)` has been added to explicitly indicate that an `inout` variable will not be modified in the function. This is only one way to address the specific validation error in #4698. In general it seems that DXC does some more extensive checks on actual struct fields (as opposed to observing arbitrary struct writes), so that will be the next step. --- .../reverse-inout-param-custom-derivative.slang | 2 +- tests/bugs/gl-33-ext.slang | 2 +- tests/bugs/optional.slang | 2 +- tests/diagnostics/inout-never-written.slang | 48 ++++++++++++++++++++++ tests/hlsl-intrinsic/matrix-double.slang | 4 ++ tests/hlsl-intrinsic/size-of/align-of-3.slang | 4 +- tests/hlsl-intrinsic/size-of/size-of-3.slang | 4 +- ...struct-field-no-initializer-complex-types.slang | 9 ++-- tests/language-feature/unsized-array.slang | 1 + tests/pipeline/ray-tracing/trace-ray-inline.slang | 6 ++- 10 files changed, 68 insertions(+), 14 deletions(-) create mode 100644 tests/diagnostics/inout-never-written.slang (limited to 'tests') diff --git a/tests/autodiff/reverse-inout-param-custom-derivative.slang b/tests/autodiff/reverse-inout-param-custom-derivative.slang index 8769a33c7..c4549e37b 100644 --- a/tests/autodiff/reverse-inout-param-custom-derivative.slang +++ b/tests/autodiff/reverse-inout-param-custom-derivative.slang @@ -5,7 +5,7 @@ //TEST_INPUT:ubuffer(data=[0], stride=4):out,name=outputBuffer RWStructuredBuffer outputBuffer; -float rng(inout int state, float x) +float rng(int state, float x) { return state + x; } diff --git a/tests/bugs/gl-33-ext.slang b/tests/bugs/gl-33-ext.slang index fccde99a3..6df9f286a 100644 --- a/tests/bugs/gl-33-ext.slang +++ b/tests/bugs/gl-33-ext.slang @@ -4,5 +4,5 @@ public struct A { public int state; - [mutating] public int next() { return state; } + public int next() { return state; } }; diff --git a/tests/bugs/optional.slang b/tests/bugs/optional.slang index 3512ba29f..7a220b23d 100644 --- a/tests/bugs/optional.slang +++ b/tests/bugs/optional.slang @@ -14,7 +14,7 @@ struct P } struct Tr { - int test(T t, inout P p) + int test(T t, P p) { const IFoo hit = p.f; let castResult = hit as S; diff --git a/tests/diagnostics/inout-never-written.slang b/tests/diagnostics/inout-never-written.slang new file mode 100644 index 000000000..0bc59ecf3 --- /dev/null +++ b/tests/diagnostics/inout-never-written.slang @@ -0,0 +1,48 @@ +//TEST:SIMPLE(filecheck=CHK): -target spirv + +struct State +{ + float3 v; + float3 n; + int rnd; +}; + +//CHK-DAG: ([[# @LINE + 1]]): warning 41022: inout parameter 'x' is never written to +void int_never_assigned(inout int x) {} + +//CHK-DAG: ([[# @LINE + 1]]): warning 41022: inout parameter 'state' is never written to +void state_never_assigned(inout State state, inout float v) +{ + v = state.v.x; +} + +void state_assigned(inout State state) +{ + state.rnd = (int) dot(state.v, state.n); +} + +struct A +{ + int state; + + //CHK-DAG: ([[# @LINE + 1]]): warning 41023: method marked `[mutable]` but never modifies `this` + [mutating] int next() { return state; } + + [mutating] int progress() + { + unmodified(state); + return state; + } +}; + +__generic +struct B +{ + int state; + + //CHK-DAG: ([[# @LINE + 1]]): warning 41023: method marked `[mutable]` but never modifies `this` + [mutating] int next() { return state; } +}; + +//CHK-NOT: warning 41022 +//CHK-NOT: warning 41023 \ No newline at end of file diff --git a/tests/hlsl-intrinsic/matrix-double.slang b/tests/hlsl-intrinsic/matrix-double.slang index 31ec54f8e..6ed1b42bf 100644 --- a/tests/hlsl-intrinsic/matrix-double.slang +++ b/tests/hlsl-intrinsic/matrix-double.slang @@ -46,6 +46,8 @@ IntMatrix makeIntMatrix(int v) void test1(inout FloatMatrix ft, inout FloatMatrix f, int idx) { + unmodified(f); + // fmod ft += FloatMatrix(IntMatrix(((f % makeFloatMatrix(0.11f)) * makeFloatMatrix(100)) + makeFloatMatrix(0.5))); @@ -112,6 +114,8 @@ void test1(inout FloatMatrix ft, inout FloatMatrix f, int idx) void test2(inout FloatMatrix ft, inout FloatMatrix f) { + unmodified(f); + ft += pow(makeFloatMatrix(0.5), f); ft += smoothstep(makeFloatMatrix(0.2), makeFloatMatrix(0.7), f); diff --git a/tests/hlsl-intrinsic/size-of/align-of-3.slang b/tests/hlsl-intrinsic/size-of/align-of-3.slang index 28665b5ab..079de9241 100644 --- a/tests/hlsl-intrinsic/size-of/align-of-3.slang +++ b/tests/hlsl-intrinsic/size-of/align-of-3.slang @@ -10,7 +10,7 @@ RWStructuredBuffer outputBuffer; -int getParamSize(inout uint3 v) +int getParamSize(uint3 v) { return alignof(v); } @@ -19,7 +19,7 @@ int getParamSize(inout uint3 v) // With "natural" layout size must be the same across all targets. // The parameter passing semantic is "value in/value out", so the size // will always be the size of the *value* regardless. So uint3 in this case. -int getParamSizeWithDirection(inout uint3 v) +int getParamSizeWithDirection(uint3 v) { return alignof(v); } diff --git a/tests/hlsl-intrinsic/size-of/size-of-3.slang b/tests/hlsl-intrinsic/size-of/size-of-3.slang index 8a211867f..7e55bd320 100644 --- a/tests/hlsl-intrinsic/size-of/size-of-3.slang +++ b/tests/hlsl-intrinsic/size-of/size-of-3.slang @@ -10,7 +10,7 @@ RWStructuredBuffer outputBuffer; -int getParamSize(inout uint3 v) +int getParamSize(uint3 v) { return sizeof(v); } @@ -19,7 +19,7 @@ int getParamSize(inout uint3 v) // With "natural" layout size must be the same across all targets. // The parameter passing semantic is "value in/value out", so the size // will always be the size of the *value* regardless. So uint3 in this case. -int getParamSizeWithDirection(inout uint3 v) +int getParamSizeWithDirection(uint3 v) { return sizeof(v); } diff --git a/tests/language-feature/struct-field-initializers/struct-field-no-initializer-complex-types.slang b/tests/language-feature/struct-field-initializers/struct-field-no-initializer-complex-types.slang index a55c4725d..66a3a2c64 100644 --- a/tests/language-feature/struct-field-initializers/struct-field-no-initializer-complex-types.slang +++ b/tests/language-feature/struct-field-initializers/struct-field-no-initializer-complex-types.slang @@ -6,7 +6,7 @@ //TEST_INPUT:ubuffer(data=[0 0], stride=4):out,name=outputBuffer RWStructuredBuffer outputBuffer; -struct DefaultData +struct DefaultData : IDefaultInitializable { static const int2 val = int2(0, 1); float2 size; @@ -16,13 +16,10 @@ struct DefaultData extension DefaultData { - int someGet() - { - return val.x; - } + int someGet() { return val.x; } } -int loadDefaultData(inout DefaultData noInit) +int loadDefaultData(DefaultData noInit) { outputBuffer[1] = 1; return noInit.someGet(); diff --git a/tests/language-feature/unsized-array.slang b/tests/language-feature/unsized-array.slang index c4479867e..19ab0bd63 100644 --- a/tests/language-feature/unsized-array.slang +++ b/tests/language-feature/unsized-array.slang @@ -17,6 +17,7 @@ int test(int arr[]) int test2(inout T arr[]) { + unmodified(arr); return arr.getCount(); } diff --git a/tests/pipeline/ray-tracing/trace-ray-inline.slang b/tests/pipeline/ray-tracing/trace-ray-inline.slang index 009dffbc7..b8688c2f5 100644 --- a/tests/pipeline/ray-tracing/trace-ray-inline.slang +++ b/tests/pipeline/ray-tracing/trace-ray-inline.slang @@ -57,6 +57,7 @@ void myTriangleClosestHit(inout MyRayPayload payload) // bool myTriangleAnyHit(inout MyRayPayload payload) { + unmodified(payload); return true; } @@ -75,6 +76,7 @@ void myProceduralClosestHit(inout MyRayPayload payload, MyProceduralHitAttrs att } bool myProceduralAnyHit(inout MyRayPayload payload) { + unmodified(payload); return true; } @@ -87,6 +89,8 @@ bool myProceduralAnyHit(inout MyRayPayload payload) // bool myProceduralIntersection(inout float tHit, inout MyProceduralHitAttrs hitAttrs) { + unmodified(tHit); + unmodified(hitAttrs); return true; } @@ -180,4 +184,4 @@ void main(uint3 tid : SV_DispatchThreadID) } resultBuffer[index] = payload.value; -} \ No newline at end of file +} -- cgit v1.2.3