diff options
| -rw-r--r-- | source/slang/lower.cpp | 138 | ||||
| -rw-r--r-- | tests/rewriter/glslang-bug-988-workaround.frag | 58 | ||||
| -rw-r--r-- | tests/rewriter/glslang-bug-988-workaround.slang | 6 | ||||
| -rw-r--r-- | tests/rewriter/resources-in-structs.glsl | 3 |
4 files changed, 192 insertions, 13 deletions
diff --git a/source/slang/lower.cpp b/source/slang/lower.cpp index 9c2c38b60..d634b114b 100644 --- a/source/slang/lower.cpp +++ b/source/slang/lower.cpp @@ -733,6 +733,18 @@ struct LoweringVisitor return result; } + RefPtr<ExpressionSyntaxNode> moveTemp(RefPtr<ExpressionSyntaxNode> expr) + { + RefPtr<Variable> varDecl = new Variable(); + varDecl->Name.Content = generateName(); + varDecl->Type.type = expr->Type.type; + varDecl->Expr = expr; + + addDecl(varDecl); + + return createVarRef(expr->Position, varDecl); + } + // The idea of this function is to take an expression that we plan to // use/evaluate more than once, and if needed replace it with a // reference to a temporary (initialized with the expr) so that it @@ -769,14 +781,7 @@ struct LoweringVisitor // TODO: handle the tuple cases here... // In the general case, though, we need to introduce a temporary - RefPtr<Variable> varDecl = new Variable(); - varDecl->Name.Content = generateName(); - varDecl->Type.type = expr->Type.type; - varDecl->Expr = expr; - - addDecl(varDecl); - - return createVarRef(expr->Position, varDecl); + return moveTemp(expr); } // Similar to the above, this ensures that an l-value expression @@ -1289,11 +1294,11 @@ struct LoweringVisitor // Default case: just reconstrut a subscript expr auto loweredExpr = new IndexExpressionSyntaxNode(); - loweredExpr->Type.type = getSubscripResultType(baseExpr->Type.type); +loweredExpr->Type.type = getSubscripResultType(baseExpr->Type.type); - loweredExpr->BaseExpression = baseExpr; - loweredExpr->IndexExpression = indexExpr; - return loweredExpr; +loweredExpr->BaseExpression = baseExpr; +loweredExpr->IndexExpression = indexExpr; +return loweredExpr; } } @@ -1358,6 +1363,84 @@ struct LoweringVisitor return expr; } + bool needGlslangBug988Workaround( + RefPtr<ExpressionSyntaxNode> inExpr) + { + switch (getTarget()) + { + default: + return false; + + case CodeGenTarget::GLSL: + break; + } + + // There are two conditions we care about here: + // + // (1) is the *type* of the expression something that needs the WAR + // (2) does the expression reference a constant-buffer member? + // + + // Issue (1): is the type of the expression something that needs the WAR? + + auto exprType = inExpr->Type.type; + exprType = unwrapArray(exprType); + + if (!isStructType(exprType)) + return false; + + + // Issue (2): does the expression reference a constant-buffer member? + + auto expr = inExpr; + for (;;) + { + if (auto memberRefExpr = expr.As<MemberExpressionSyntaxNode>()) + { + expr = memberRefExpr->BaseExpression; + continue; + } + + if (auto derefExpr = expr.As<DerefExpr>()) + { + expr = derefExpr->base; + continue; + } + + if (auto subscriptExpr = expr.As<IndexExpressionSyntaxNode>()) + { + expr = subscriptExpr->BaseExpression; + continue; + } + + break; + } + + if (auto varExpr = expr.As<VarExpressionSyntaxNode>()) + { + auto declRef = varExpr->declRef; + if (!declRef) + return false; + + if (auto varDeclRef = declRef.As<Variable>()) + { + auto varType = GetType(varDeclRef); + + while (auto arrayType = varType->As<ArrayExpressionType>()) + { + varType = arrayType->BaseType; + } + + if (auto constantBufferType = varType->As<ConstantBufferType>()) + { + return true; + } + } + } + + return false; + } + void addArgs( ExprWithArgsBase* callExpr, RefPtr<ExpressionSyntaxNode> argExpr) @@ -1382,6 +1465,17 @@ struct LoweringVisitor } else { + // This should be the default case where we have a perfectly + // ordinary expression, but we need to work around a glslang + // but here, where passing a member of a `uniform` block + // that has `struct` type directly to a function call causes + // invalid SPIR-V to be generated. + if (needGlslangBug988Workaround(argExpr)) + { + argExpr = moveTemp(argExpr); + } + + // Here's the actual default case where we just add an argment callExpr->Arguments.Add(argExpr); } } @@ -2981,8 +3075,28 @@ struct LoweringVisitor } } + AggTypeDecl* isStructType(RefPtr<ExpressionType> type) + { + if (type->As<BasicExpressionType>()) return nullptr; + else if (type->As<VectorExpressionType>()) return nullptr; + else if (type->As<MatrixExpressionType>()) return nullptr; + else if (type->As<ResourceType>()) return nullptr; + else if (type->As<BuiltinGenericType>()) return nullptr; + else if (auto declRefType = type->As<DeclRefType>()) + { + if (auto aggTypeDeclRef = declRefType->declRef.As<AggTypeDecl>()) + { + return aggTypeDeclRef.getDecl(); + } + } + + return nullptr; + } + bool isImportedStructType(RefPtr<ExpressionType> type) { + // TODO: make this use `isStructType` above + if (type->As<BasicExpressionType>()) return false; else if (type->As<VectorExpressionType>()) return false; else if (type->As<MatrixExpressionType>()) return false; diff --git a/tests/rewriter/glslang-bug-988-workaround.frag b/tests/rewriter/glslang-bug-988-workaround.frag new file mode 100644 index 000000000..8c9692aed --- /dev/null +++ b/tests/rewriter/glslang-bug-988-workaround.frag @@ -0,0 +1,58 @@ +#version 450 +//TEST:COMPARE_GLSL: + +// Test workaround for glslang issue #988 +// (https://github.com/KhronosGroup/glslang/issues/988) + + +#if defined(__SLANG__) + + +__import glslang_bug_988_workaround; + +uniform U +{ + Foo foo; +}; + +vec4 doIt(Foo foo) +{ + return foo.bar; +} + +layout(location = 0) +out vec4 result; + +void main() +{ + result = doIt(foo); +} + +#else + +struct Foo +{ + vec4 bar; +}; + +layout(binding = 0) +uniform U +{ + Foo foo; +}; + +vec4 doIt(Foo foo) +{ + return foo.bar; +} + +layout(location = 0) +out vec4 result; + +void main() +{ + Foo SLANG_tmp_0 = foo; + result = doIt(SLANG_tmp_0); +} + +#endif diff --git a/tests/rewriter/glslang-bug-988-workaround.slang b/tests/rewriter/glslang-bug-988-workaround.slang new file mode 100644 index 000000000..841375923 --- /dev/null +++ b/tests/rewriter/glslang-bug-988-workaround.slang @@ -0,0 +1,6 @@ +//TEST_IGNORE_FILE: + +struct Foo +{ + float4 bar; +}; diff --git a/tests/rewriter/resources-in-structs.glsl b/tests/rewriter/resources-in-structs.glsl index 6273e8720..206e4a8d8 100644 --- a/tests/rewriter/resources-in-structs.glsl +++ b/tests/rewriter/resources-in-structs.glsl @@ -55,8 +55,9 @@ out vec4 color; void main() { + Material SLANG_tmp_0 = m; color = evaluateMaterial( - m, + SLANG_tmp_0, SLANG_parameterBlock_U_m_t, SLANG_parameterBlock_U_m_s, uv); } |