diff options
| author | Tim Foley <tfoleyNV@users.noreply.github.com> | 2020-10-15 13:13:49 -0700 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2020-10-15 13:13:49 -0700 |
| commit | d3e255b31b39c9a8979a60023269567078d9dce3 (patch) | |
| tree | d6915c1d0219eeea4d6fc257f6504ee4522840a4 | |
| parent | 4149bf2c9ce4e9e1a8457da8a497cdb137eaad7c (diff) | |
Fix a bug in IR lowering (#1578)
The basic problem here is that when a function has multiple declarations with matching signatures (e.g., a forward declaration and then a later definition with a body), the IR lowering logic would lower all declarations whenever the first one was encountered, but then would only register an IR value as the lowered version of the first declaration. Other matching declarations would then run the risk of being lowered again, and in the case where they included features like loops with break/continue labels, that would create the risk of keys getting inserted into certain dictionaries more than one, leading to exceptions.
This change ensures that when lowering a function that has multiple matching declarations to IR, we register an IR value for all of those declarations and not just the first.
I have added a test case that leads to a crash without this change, to ensure that we don't introduce a regression down the line.
| -rw-r--r-- | source/slang/slang-lower-to-ir.cpp | 13 | ||||
| -rw-r--r-- | tests/bugs/multiple-definitions.slang | 22 |
2 files changed, 35 insertions, 0 deletions
diff --git a/source/slang/slang-lower-to-ir.cpp b/source/slang/slang-lower-to-ir.cpp index e9381268b..ad7cec773 100644 --- a/source/slang/slang-lower-to-ir.cpp +++ b/source/slang/slang-lower-to-ir.cpp @@ -7154,6 +7154,19 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo> auto funcDecl = as<FunctionDeclBase>(dd); SLANG_ASSERT(funcDecl); lowerFuncDecl(funcDecl); + + // Note: Because we are iterating over multiple declarations, + // but only one will be registered as the value for `decl` + // in the global mapping by `ensureDecl()`, we have to take + // responsibility here for registering a lowered value + // for the remaining (non-primary) declarations. + // + // It doesn't really matter which one we register here, because + // they will all have the same mangled name in the IR, but we + // default to the `result` that is returned from this visitor, + // so that all the declarations share the same IR representative. + // + setGlobalValue(context->shared, funcDecl, result); } return result; } diff --git a/tests/bugs/multiple-definitions.slang b/tests/bugs/multiple-definitions.slang new file mode 100644 index 000000000..5361c97bc --- /dev/null +++ b/tests/bugs/multiple-definitions.slang @@ -0,0 +1,22 @@ +// multiple-definitions.slang +//TEST:SIMPLE:-entry main -o multiple-definitions.hlsl + +__specialized_for_target(hlsl) +int a(int x) +{ + int r = 0; + for(int i = 0; i < x; ++i) ++r; + return r; +} + +__specialized_for_target(glsl) +int a(int x) +{ + int r = 0; + for(int i = 0; i < x; ++i) ++r; + return r; +} + +[shader("compute")] +void main() +{} |
