summaryrefslogtreecommitdiffstats
path: root/source
diff options
context:
space:
mode:
authorTim Foley <tfoley@nvidia.com>2017-07-23 18:22:30 -0700
committerTim Foley <tfoley@nvidia.com>2017-07-23 18:37:52 -0700
commitd4cfe5781284551d7c5ccf2cf1d28a86211bb1df (patch)
tree977c7923635dcbb57716a38cde82b75736841936 /source
parent376e61abe84ed386df7aa28867c1aeb5f52881ca (diff)
Work around glslang issue 988
The basic bug there is that if you have a member of `struct` type in a `uniform` block and then pass a reference to that member directly to a call: ``` struct Foo { vec4 bar; }; uniform U { Foo foo; }; void main() { doSomething(foo); } ``` then glslang generates invalid SPIR-V which seems to cause an issue for some drivers. This change works around the problem by detecting cases where an argument to a function call is a reference to `uniform` block member (of `struct` type) and then rewrites the code to move that value to a temporary before the call.
Diffstat (limited to 'source')
-rw-r--r--source/slang/lower.cpp138
1 files changed, 126 insertions, 12 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;