From 05547e25353dd797791c2937679468d529d832d5 Mon Sep 17 00:00:00 2001 From: venkataram-nv Date: Tue, 16 Jul 2024 14:54:53 -0700 Subject: Warnings function parameters (#4626) * Handle out/inout functions with separate consideration * Fixing bug with passing aliasable instructions * Handle autodiff functions (fwd and rev) in warning system * Handling interface methods * Handling ref parameters like out/inout * Temporary fix to remaining bugs * Refactoring methods and tests * Recursive check for empty structs * Using default initializable interface in tests * Resolving CI fail --- tests/autodiff/material2/MxWeights.slang | 6 + tests/bugs/generic-param-cast.slang | 2 +- tests/bugs/specialize-existential-in-generic.slang | 2 +- tests/compute/assoctype-func-param.slang | 4 +- tests/compute/assoctype-nested-lookup.slang | 4 +- tests/compute/dynamic-dispatch-17.slang | 4 +- tests/compute/empty-struct2.slang | 6 +- tests/compute/generic-closer.slang | 4 +- tests/compute/generic-default-arg.slang | 4 +- tests/compute/transitive-interface.slang | 4 +- tests/cuda/cuda-array-layout.slang | 2 +- .../interfaces/anyvalue-size-validation.slang | 4 +- tests/diagnostics/uninitialized-globals.slang | 48 ++++ .../uninitialized-local-variables.slang | 151 ++++++++++++ .../diagnostics/uninitialized-out-parameters.slang | 51 ++++ tests/diagnostics/uninitialized-out.slang.expected | 20 -- .../diagnostics/uninitialized-use-functions.slang | 113 +++++++++ tests/diagnostics/uninitialized.slang | 263 --------------------- tests/language-feature/types/is-on-type.slang | 4 +- 19 files changed, 391 insertions(+), 305 deletions(-) create mode 100644 tests/diagnostics/uninitialized-globals.slang create mode 100644 tests/diagnostics/uninitialized-local-variables.slang create mode 100644 tests/diagnostics/uninitialized-out-parameters.slang delete mode 100644 tests/diagnostics/uninitialized-out.slang.expected create mode 100644 tests/diagnostics/uninitialized-use-functions.slang delete mode 100644 tests/diagnostics/uninitialized.slang (limited to 'tests') diff --git a/tests/autodiff/material2/MxWeights.slang b/tests/autodiff/material2/MxWeights.slang index 1e9c7d36c..244d97c73 100644 --- a/tests/autodiff/material2/MxWeights.slang +++ b/tests/autodiff/material2/MxWeights.slang @@ -3,6 +3,12 @@ public struct MxWeights { public float3 weights[TBsdfCount]; + + public __init() + { + for (int i = 0; i < TBsdfCount; i++) + weights[i] = float3(0.0f); + } } public interface IMxLayeredMaterialData diff --git a/tests/bugs/generic-param-cast.slang b/tests/bugs/generic-param-cast.slang index 20b30a433..e2dbcc285 100644 --- a/tests/bugs/generic-param-cast.slang +++ b/tests/bugs/generic-param-cast.slang @@ -8,7 +8,7 @@ struct A int f() { return I; } }; -struct B +struct B : IDefaultInitializable { A a; }; diff --git a/tests/bugs/specialize-existential-in-generic.slang b/tests/bugs/specialize-existential-in-generic.slang index 695c7bc29..cb05aea14 100644 --- a/tests/bugs/specialize-existential-in-generic.slang +++ b/tests/bugs/specialize-existential-in-generic.slang @@ -18,7 +18,7 @@ struct Impl : IFoo Assoc getValue() { Assoc r; return r; } } -struct GenType +struct GenType : IDefaultInitializable { T obj; int doThing() diff --git a/tests/compute/assoctype-func-param.slang b/tests/compute/assoctype-func-param.slang index 830cc00cc..6d6bf5b6d 100644 --- a/tests/compute/assoctype-func-param.slang +++ b/tests/compute/assoctype-func-param.slang @@ -37,7 +37,7 @@ struct GenStruct : IBase U.RetT test(U.RetT val) { - U obj; + U obj = U(); U.SubTypeT sb = obj.setVal(val); return obj.getVal(sb); } @@ -50,4 +50,4 @@ void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID) /* wrong error message for the following line */ float outVal = test >(inVal); outputBuffer[tid] = outVal.x; -} \ No newline at end of file +} diff --git a/tests/compute/assoctype-nested-lookup.slang b/tests/compute/assoctype-nested-lookup.slang index b1ca75d49..64a5d29a5 100644 --- a/tests/compute/assoctype-nested-lookup.slang +++ b/tests/compute/assoctype-nested-lookup.slang @@ -12,7 +12,7 @@ interface IFoo : IDefaultInitializable }; -struct FooPair : IFoo +struct FooPair : IFoo, IDefaultInitializable { T a; T.Bar b; @@ -41,4 +41,4 @@ void computeMain(uint3 dispatchThreadID: SV_DispatchThreadID) { FooPair.Bar pair; test(pair); -} \ No newline at end of file +} diff --git a/tests/compute/dynamic-dispatch-17.slang b/tests/compute/dynamic-dispatch-17.slang index bc2b9a6d9..e89482a5b 100644 --- a/tests/compute/dynamic-dispatch-17.slang +++ b/tests/compute/dynamic-dispatch-17.slang @@ -52,7 +52,7 @@ struct FloatVal : IInterface float val; float run() { - Z z; + Z z = Z(); return val + z.get(); } }; @@ -62,7 +62,7 @@ struct Float4Val : IInterface Float4Struct val; float run() { - Z z; + Z z = Z(); return val.val.x + val.val.y + z.get(); } }; diff --git a/tests/compute/empty-struct2.slang b/tests/compute/empty-struct2.slang index 27e587b42..303cfd234 100644 --- a/tests/compute/empty-struct2.slang +++ b/tests/compute/empty-struct2.slang @@ -25,8 +25,8 @@ struct EmptyS : IEmptyS struct Empty : IInterface { typedef TT T; - TT value; - float a; + TT value = TT(); + float a = 0; TT getT() { return value; @@ -51,4 +51,4 @@ void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID) Empty obj; test(obj); outputBuffer[dispatchThreadID.x] = dispatchThreadID.x; -} \ No newline at end of file +} diff --git a/tests/compute/generic-closer.slang b/tests/compute/generic-closer.slang index 377b42dab..c497b14d0 100644 --- a/tests/compute/generic-closer.slang +++ b/tests/compute/generic-closer.slang @@ -13,7 +13,7 @@ struct Gen0 : IGetter }; struct Gen1 : IGetter { - TGetter g; + TGetter g = TGetter(); int get() { return g.get(); } }; @@ -39,4 +39,4 @@ void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID) int b = 5; if (a< b && b > a) outputBuffer[dispatchThreadID.x] = (g.get() >> 1) + g2.get() + g3.get(); -} \ No newline at end of file +} diff --git a/tests/compute/generic-default-arg.slang b/tests/compute/generic-default-arg.slang index 8762f5e8a..3a3a4a5b8 100644 --- a/tests/compute/generic-default-arg.slang +++ b/tests/compute/generic-default-arg.slang @@ -31,7 +31,7 @@ struct Impl2 : ITest __generic struct GenStruct { - T obj; + T obj = T(); }; int test(GenStruct gs, int val) @@ -50,4 +50,4 @@ void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID) outVal += test(gs, tid); outputBuffer[tid] = outVal; -} \ No newline at end of file +} diff --git a/tests/compute/transitive-interface.slang b/tests/compute/transitive-interface.slang index e4d7db91d..d8b167bd7 100644 --- a/tests/compute/transitive-interface.slang +++ b/tests/compute/transitive-interface.slang @@ -47,7 +47,7 @@ struct AssocImpl : IAssoc int testAdd2(T assoc) { - T.AT obj; + T.AT obj = T.AT(); return obj.addf(1, 1); } @@ -69,4 +69,4 @@ void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID) outVal += testSub(s1, outVal); outputBuffer[dispatchThreadID.x] = outVal; -} \ No newline at end of file +} diff --git a/tests/cuda/cuda-array-layout.slang b/tests/cuda/cuda-array-layout.slang index 60f318962..b99ab84c8 100644 --- a/tests/cuda/cuda-array-layout.slang +++ b/tests/cuda/cuda-array-layout.slang @@ -10,7 +10,7 @@ struct PadLadenStruct }; // This is to check if the last half can be inserted 'inside' the spare padding of a. It should not be -struct StructWithArray +struct StructWithArray : IDefaultInitializable { PadLadenStruct a[1]; uint8_t b; diff --git a/tests/diagnostics/interfaces/anyvalue-size-validation.slang b/tests/diagnostics/interfaces/anyvalue-size-validation.slang index 1ebf7f4c3..f2ed52d0c 100644 --- a/tests/diagnostics/interfaces/anyvalue-size-validation.slang +++ b/tests/diagnostics/interfaces/anyvalue-size-validation.slang @@ -26,6 +26,6 @@ RWStructuredBuffer output; [numthreads(4, 1, 1)] void main() { - S s; + S s = S(); output[0] = test(s).a; -} \ No newline at end of file +} diff --git a/tests/diagnostics/uninitialized-globals.slang b/tests/diagnostics/uninitialized-globals.slang new file mode 100644 index 000000000..314881a80 --- /dev/null +++ b/tests/diagnostics/uninitialized-globals.slang @@ -0,0 +1,48 @@ +//TEST:SIMPLE(filecheck=CHK): -target spirv + +// Using groupshared variables +groupshared float4 gsConstexpr = float4(1.0f); +groupshared float4 gsUndefined; + +// OK +float use_constexpr_initialized_gs() +{ + return gsConstexpr.x; +} + +float use_undefined_gs() +{ + //CHK-DAG: warning 41017: use of uninitialized global variable 'gsUndefined' + return gsUndefined.x; +} + +// Using static variables +static const float cexprInitialized = 1.0f; +static float writtenNever; +static float writtenLater; + +// OK +float use_initialized_static() +{ + return cexprInitialized; +} + +// Should detect this and treat it as a store +void write_to_later() +{ + writtenLater = 1.0f; +} + +float use_never_written() +{ + //CHK-DAG: warning 41017: use of uninitialized global variable 'writtenNever' + return writtenNever; +} + +// OK because of prior store +float use_later_writte() +{ + return writtenLater; +} + +//CHK-NOT: warning 41017 diff --git a/tests/diagnostics/uninitialized-local-variables.slang b/tests/diagnostics/uninitialized-local-variables.slang new file mode 100644 index 000000000..5a2119f53 --- /dev/null +++ b/tests/diagnostics/uninitialized-local-variables.slang @@ -0,0 +1,151 @@ +//TEST:SIMPLE(filecheck=CHK): -target spirv + +// Should not warn here (unconditionalBranch) +float3 unconditional(int mode) +{ + float f(float) { return 1; } + + float k0; + float k1; + + if (mode == 1) + { + k1 = 1; + k0 = 1; + + const float w = k1 * f(1); + k0 = 4.0f * k0 * w; + k1 = 2.0f * k1 * w; + } + + return k0 + k1; +} + +// Warn here for branches using the variables +int conditional() +{ + int k; + //CHK-DAG: warning 41016: use of uninitialized variable 'k' + return (k > 0); +} + +// Using unitialized values +int use_undefined_value(int k) +{ + int x; + x += k; + //CHK-DAG: warning 41016: use of uninitialized variable 'x' + return x; +} + +// Returning uninitialized values +__generic +T generic_undefined_return() +{ + T x; + //CHK-DAG: warning 41016: use of uninitialized variable 'x' + return x; +} + +// Array variables +float undefined_array() +{ + float array[2]; + //CHK-DAG: warning 41016: use of uninitialized variable 'array' + return array[0]; +} + +float filled_array(int mode) +{ + float array[2]; + array[0] = 1.0f; + return array[0]; +} + +// Structs and nested structs +struct Data +{ + float value; +}; + +struct NestedData +{ + Data data; +}; + +// No warnings here, even thought autodiff generates +// IR which frequently returns undefined values +struct DiffStruct : IDifferentiable +{ + Data data; + float x; +} + +// Same story here +[ForwardDifferentiable] +DiffStruct differentiable(float x) +{ + DiffStruct ds; + ds.x = x; + return ds; +} + +// Empty structures should not generate diagnostics +// for empty default constructors +struct EmptyStruct +{ + __init() {} +}; + +// No warnings for empty structs even without __init() +struct NonEmptyStruct +{ + int field; + + __init() + { + field = 1; + } +}; + +// No warnings even when __init() is not specified +struct NoDefault +{ + int f(int i) + { + return i; + } +}; + +// Constructing the above structs +int constructors() +{ + EmptyStruct empty; + NoDefault no_default; + return no_default.f(1); +} + +// Using struct fields and nested structs +float structs() +{ + Data inputData = Data(1.0); + + float undefVar; + Data undefData; + NestedData nestedData; + + float result = inputData.value; + + //CHK-DAG: warning 41016: use of uninitialized variable 'undefVar' + result += undefVar; + + //CHK-DAG: warning 41016: use of uninitialized variable 'undefData' + result += undefData.value; + + //CHK-DAG: warning 41016: use of uninitialized variable 'nestedData' + result += nestedData.data.value; + + return result; +} + +//CHK-NOT: warning 41016 diff --git a/tests/diagnostics/uninitialized-out-parameters.slang b/tests/diagnostics/uninitialized-out-parameters.slang new file mode 100644 index 000000000..9714a4b76 --- /dev/null +++ b/tests/diagnostics/uninitialized-out-parameters.slang @@ -0,0 +1,51 @@ +//TEST:SIMPLE(filecheck=CHK): -target spirv + +// Using before assigning +float regular_undefined_use(out float3 v) +{ + //CHK-DAG: warning 41015: use of uninitialized out parameter 'v' + float r = v.x + 1.0; + + //CHK-DAG: warning 41018: returning without initializing out parameter 'v' + return r; +} + +// Returning before assigning +float returning_undefined_use(out float x) +{ + //CHK-DAG: warning 41018: returning without initializing out parameter 'x' + return 0; +} + +// Implicit, still returning before assigning +void implicit_undefined_use(out float x) +{ + //CHK-DAG: warning 41018: returning without initializing out parameter 'x' +} + +// Warn on potential return paths +void control_flow_undefined(bool b, out float y) +{ + if(b) + { + //CHK-DAG: warning 41018: returning without initializing out parameter 'y' + return; + } + y = 0; + return; +} + +// No warnings if all paths are fine +void control_flow_defined(bool b, out float y) +{ + if(b) + { + unused(y); + return; + } + y = 0; + return; +} + +//CHK-NOT: warning 41015 +//CHK-NOT: warning 41018 diff --git a/tests/diagnostics/uninitialized-out.slang.expected b/tests/diagnostics/uninitialized-out.slang.expected deleted file mode 100644 index 5846c12df..000000000 --- a/tests/diagnostics/uninitialized-out.slang.expected +++ /dev/null @@ -1,20 +0,0 @@ -result code = -1 -standard error = { -tests/diagnostics/uninitialized-out.slang(6): error 41015: use of uninitialized value 'v' - float r = v.x + 1.0; - ^ -tests/diagnostics/uninitialized-out.slang(8): warning 41016: returning without initializing out parameter 'v' - return r; - ^~~~~~ -tests/diagnostics/uninitialized-out.slang(14): warning 41016: returning without initializing out parameter 'x' - return 0; - ^~~~~~ -tests/diagnostics/uninitialized-out.slang(18): warning 41016: returning without initializing out parameter 'x' -void baz(out float x) {} - ^ -tests/diagnostics/uninitialized-out.slang(25): warning 41016: returning without initializing out parameter 'y' - return; - ^~~~~~ -} -standard output = { -} diff --git a/tests/diagnostics/uninitialized-use-functions.slang b/tests/diagnostics/uninitialized-use-functions.slang new file mode 100644 index 000000000..82e7a04a8 --- /dev/null +++ b/tests/diagnostics/uninitialized-use-functions.slang @@ -0,0 +1,113 @@ +//TEST:SIMPLE(filecheck=CHK): -target spirv + +// Both out and inout parameters +// should have this treated as writes +void out_test(int tmp, out int x) +{ + x = tmp; +} + +// Permuting arguments to ensure that +// the correct argument is checked +void inout_test(inout int x, int tmp) +{ + x = tmp; +} + +// __ref parameters should also be fine +void ref_test(__ref int x, int tmp) +{ + x = tmp; +} + +void undefined_function_use() +{ + int x; + int tmp = 1; + + //CHK-DAG: warning 41016: use of uninitialized variable 'x' + out_test(x, tmp); + + //CHK-DAG: warning 41016: use of uninitialized variable 'x' + inout_test(tmp, x); +} + +void out_function_use() +{ + int x; + int tmp = 1; + + // Acts as a write + out_test(tmp, x); + + // Rest are fine now + out_test(x, tmp); + inout_test(tmp, x); +} + +void inout_function_use() +{ + int x; + int tmp = 1; + + // Acts as a write + inout_test(x, tmp); + + // Rest are fine now + out_test(x, tmp); + inout_test(tmp, x); +} + +void ref_function_use() +{ + int x; + int tmp = 1; + + // Acts as a write + ref_test(x, tmp); + + // Rest are fine now + out_test(x, tmp); + inout_test(tmp, x); +} + +// Likewise for generic functions +static int ord_gen_result; + +__generic +void ordinary_generic(T x) +{ + ord_gen_result = __slang_noop_cast(x); +} + +__generic +void unordinary_generic(out T x) +{ + x = __slang_noop_cast(1); +} + +void undefined_generic_use() +{ + int x; + + //CHK-DAG: warning 41016: use of uninitialized variable 'x' + ordinary_generic(x); +} + +void ok_generic_use() +{ + int x; + unordinary_generic(x); + ordinary_generic(x); +} + +// Check proper handling of aliases passed +void f() +{ + int3 dim; + + // Should have no warnings + out_test(1, dim.x); +} + +//CHK-NOT: warning 41016 diff --git a/tests/diagnostics/uninitialized.slang b/tests/diagnostics/uninitialized.slang deleted file mode 100644 index 4779f45c9..000000000 --- a/tests/diagnostics/uninitialized.slang +++ /dev/null @@ -1,263 +0,0 @@ -//TEST:SIMPLE(filecheck=CHK): -target spirv - -// TODO: -// * warn potentially uninitialized variables (control flow) -// * warn partially uninitialized variables (structs, arrays, etc.) -// * warn uninitialized fields in constructors - -/////////////////////////////////// -// Uninitialized local variables // -/////////////////////////////////// - -// Should not warn here (unconditionalBranch) -float3 unconditional(int mode) -{ - float f(float) { return 1; } - - float k0; - float k1; - - if (mode == 1) - { - k1 = 1; - k0 = 1; - - const float w = k1 * f(1); - k0 = 4.0f * k0 * w; - k1 = 2.0f * k1 * w; - } - - return k0 + k1; -} - -// Warn here for branches using the variables -int conditional() -{ - int k; - //CHK-DAG: warning 41016: use of uninitialized variable 'k' - return (k > 0); -} - -// Using unitialized values -int use_undefined_value(int k) -{ - int x; - x += k; - //CHK-DAG: warning 41016: use of uninitialized variable 'x' - return x; -} - -// Returning uninitialized values -__generic -T generic_undefined_return() -{ - T x; - //CHK-DAG: warning 41016: use of uninitialized variable 'x' - return x; -} - -// Array variables -float undefined_array() -{ - float array[2]; - //CHK-DAG: warning 41016: use of uninitialized variable 'array' - return array[0]; -} - -float filled_array(int mode) -{ - float array[2]; - array[0] = 1.0f; - return array[0]; -} - -// Structs and nested structs -struct Data -{ - float value; -}; - -struct NestedData -{ - Data data; -}; - -// No warnings here, even thought autodiff generates -// IR which frequently returns undefined values -struct DiffStruct : IDifferentiable -{ - Data data; - float x; -} - -// Same story here -[ForwardDifferentiable] -DiffStruct differentiable(float x) -{ - DiffStruct ds; - ds.x = x; - return ds; -} - -// Empty structures should not generate diagnostics -// for empty default constructors -struct EmptyStruct -{ - __init() {} -}; - -// No warnings for empty structs even without __init() -struct NonEmptyStruct -{ - int field; - - __init() - { - field = 1; - } -}; - -// No warnings even when __init() is not specified -struct NoDefault -{ - int f(int i) - { - return i; - } -}; - -// Constructing the above structs -int constructors() -{ - EmptyStruct empty; - NoDefault no_default; - return no_default.f(1); -} - -// Using struct fields and nested structs -float structs() -{ - Data inputData = Data(1.0); - - float undefVar; - Data undefData; - NestedData nestedData; - - float result = inputData.value; - - //CHK-DAG: warning 41016: use of uninitialized variable 'undefVar' - result += undefVar; - - //CHK-DAG: warning 41016: use of uninitialized variable 'undefData' - result += undefData.value; - - //CHK-DAG: warning 41016: use of uninitialized variable 'nestedData' - result += nestedData.data.value; - - return result; -} - -//////////////////////////////////// -// Uninitialized global variables // -//////////////////////////////////// - -// Using groupshared variables -groupshared float4 gsConstexpr = float4(1.0f); -groupshared float4 gsUndefined; - -// OK -float use_constexpr_initialized_gs() -{ - return gsConstexpr.x; -} - -float use_undefined_gs() -{ - //CHK-DAG: warning 41017: use of uninitialized global variable 'gsUndefined' - return gsUndefined.x; -} - -// Using static variables -static const float cexprInitialized = 1.0f; -static float writtenNever; -static float writtenLater; - -// OK -float use_initialized_static() -{ - return cexprInitialized; -} - -// Should detect this and treat it as a store -void write_to_later() -{ - writtenLater = 1.0f; -} - -float use_never_written() -{ - //CHK-DAG: warning 41017: use of uninitialized global variable 'writtenNever' - return writtenNever; -} - -// OK because of prior store -float use_later_writte() -{ - return writtenLater; -} - -////////////////////////////////// -// Uninitialized out parameters // -////////////////////////////////// - -// Using before assigning -float regular_undefined_use(out float3 v) -{ - //CHK-DAG: warning 41015: use of uninitialized out parameter 'v' - float r = v.x + 1.0; - - //CHK-DAG: warning 41018: returning without initializing out parameter 'v' - return r; -} - -// Returning before assigning -float returning_undefined_use(out float x) -{ - //CHK-DAG: warning 41018: returning without initializing out parameter 'x' - return 0; -} - -// Implicit, still returning before assigning -void implicit_undefined_use(out float x) -{ - //CHK-DAG: warning 41018: returning without initializing out parameter 'x' -} - -// Warn on potential return paths -void control_flow_undefined(bool b, out float y) -{ - if(b) - { - //CHK-DAG: warning 41018: returning without initializing out parameter 'y' - return; - } - y = 0; - return; -} - -// No warnings if all paths are fine -void control_flow_defined(bool b, out float y) -{ - if(b) - { - unused(y); - return; - } - y = 0; - return; -} - -//CHK-NOT: warning 41015 -//CHK-NOT: warning 41016 -//CHK-NOT: warning 41017 -//CHK-NOT: warning 41018 diff --git a/tests/language-feature/types/is-on-type.slang b/tests/language-feature/types/is-on-type.slang index e9dd48fcf..728f759ad 100644 --- a/tests/language-feature/types/is-on-type.slang +++ b/tests/language-feature/types/is-on-type.slang @@ -18,7 +18,7 @@ struct A : I struct B : I { - float2 f2; + float2 f2 = float2(0.0f); }; func test(T t) -> int @@ -42,4 +42,4 @@ void computeMain() B b; // CHECK: 2 outputBuffer[0] = test(b); -} \ No newline at end of file +} -- cgit v1.2.3