From 11f12cd3a1d53f988d4c9726ac4301f35dc7f01f Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Wed, 12 Jul 2017 11:56:02 -0700 Subject: Add tuple lowering logic for assignment - When assigning tuples `(a0, ...) = (b0, ...)` generate a tuple of assignments `(a0 = b0, ...)` - Given an expression statement on a tuple `(a0, ...);` generate a sequence of statements `a0; ...` --- source/slang/lower.cpp | 80 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 78 insertions(+), 2 deletions(-) (limited to 'source') diff --git a/source/slang/lower.cpp b/source/slang/lower.cpp index 02da47ccb..c7278201a 100644 --- a/source/slang/lower.cpp +++ b/source/slang/lower.cpp @@ -631,6 +631,69 @@ struct LoweringVisitor return loweredExpr; } + RefPtr createAssignExpr( + RefPtr leftExpr, + RefPtr rightExpr) + { + auto leftTuple = leftExpr.As(); + auto rightTuple = rightExpr.As(); + if (leftTuple && rightTuple) + { + RefPtr resultTuple = new TupleExpr(); + resultTuple->Type = leftExpr->Type; + + if (leftTuple->primaryExpr) + { + assert(rightTuple->primaryExpr); + + resultTuple->primaryExpr = createAssignExpr( + leftTuple->primaryExpr, + rightTuple->primaryExpr); + } + + auto elementCount = leftTuple->tupleElements.Count(); + assert(elementCount == rightTuple->tupleElements.Count()); + for (UInt ee = 0; ee < elementCount; ++ee) + { + auto leftElement = leftTuple->tupleElements[ee]; + auto rightElement = rightTuple->tupleElements[ee]; + + TupleExpr::Element resultElement; + + resultElement.tupleFieldDeclRef = leftElement.tupleFieldDeclRef; + resultElement.expr = createAssignExpr( + leftElement.expr, + rightElement.expr); + + resultTuple->tupleElements.Add(resultElement); + } + + return resultTuple; + } + else + { + assert(!leftTuple && !rightTuple); + } + + + RefPtr loweredExpr = new AssignExpr(); + loweredExpr->Type = leftExpr->Type; + loweredExpr->left = leftExpr; + loweredExpr->right = rightExpr; + return loweredExpr; + } + + RefPtr visitAssignExpr( + AssignExpr* expr) + { + auto leftExpr = lowerExpr(expr->left); + auto rightExpr = lowerExpr(expr->right); + + auto loweredExpr = createAssignExpr(leftExpr, rightExpr); + lowerExprCommon(loweredExpr, expr); + return loweredExpr; + } + void addArgs( InvokeExpressionSyntaxNode* callExpr, RefPtr argExpr) @@ -917,8 +980,21 @@ struct LoweringVisitor void addExprStmt( RefPtr expr) { - // TODO: handle cases where the `expr` cannot be directly - // represented as a single statement + // Desugar tuples in statement position + if (auto tupleExpr = expr.As()) + { + if (tupleExpr->primaryExpr) + { + addExprStmt(tupleExpr->primaryExpr); + } + for (auto ee : tupleExpr->tupleElements) + { + addExprStmt(ee.expr); + } + return; + } + + // TODO: could also desugar "operator comma" here RefPtr stmt = new ExpressionStatementSyntaxNode(); stmt->Expression = expr; -- cgit v1.2.3 From 0174470593881b5fe6c22594c9df875ab95a6735 Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Wed, 12 Jul 2017 13:26:53 -0700 Subject: Properly register error on downstream compiler failure - The old code was just doing `exit(1)` if glslang or `D3DCompile` failed, which is obviously unacceptable - The new approach adds the output to the diagnostic buffer (or invokes the callback), and tracks the error count just like any other errors --- source/slang/compiler.cpp | 16 ++++++++-------- source/slang/diagnostics.cpp | 30 ++++++++++++++++++++++++++++++ source/slang/diagnostics.h | 6 ++++++ 3 files changed, 44 insertions(+), 8 deletions(-) (limited to 'source') diff --git a/source/slang/compiler.cpp b/source/slang/compiler.cpp index 7df978707..284bb200a 100644 --- a/source/slang/compiler.cpp +++ b/source/slang/compiler.cpp @@ -236,15 +236,14 @@ namespace Slang // TODO(tfoley): need a better policy for how we translate diagnostics // back into the Slang world (although we should always try to generate // HLSL that doesn't produce any diagnostics...) - String diagnostics = (char const*) diagnosticsBlob->GetBufferPointer(); - fprintf(stderr, "%s", diagnostics.begin()); - OutputDebugStringA(diagnostics.begin()); + entryPoint->compileRequest->mSink.diagnoseRaw( + FAILED(hr) ? Severity::Error : Severity::Warning, + (char const*) diagnosticsBlob->GetBufferPointer()); diagnosticsBlob->Release(); } if (FAILED(hr)) { - // TODO(tfoley): What to do on failure? - exit(1); + return List(); } return data; } @@ -376,9 +375,10 @@ namespace Slang if (err) { - OutputDebugStringA(diagnosticOutput.Buffer()); - fprintf(stderr, "%s", diagnosticOutput.Buffer()); - exit(1); + entryPoint->compileRequest->mSink.diagnoseRaw( + Severity::Error, + diagnosticOutput.begin()); + return err; } return 0; diff --git a/source/slang/diagnostics.cpp b/source/slang/diagnostics.cpp index 0c55a94bd..870e6d172 100644 --- a/source/slang/diagnostics.cpp +++ b/source/slang/diagnostics.cpp @@ -194,6 +194,36 @@ void DiagnosticSink::diagnoseImpl(CodePosition const& pos, DiagnosticInfo const& } } +void DiagnosticSink::diagnoseRaw( + Severity severity, + char const* message) +{ + if (severity >= Severity::Error) + { + errorCount++; + } + + // Did the client supply a callback for us to use? + if( callback ) + { + // If so, pass the error string along to them + callback(message, callbackUserData); + } + else + { + // If the user doesn't have a callback, then just + // collect our diagnostic messages into a buffer + outputBuffer.append(message); + } + + if (severity >= Severity::Fatal) + { + // TODO: figure out a better policy for aborting compilation + throw InvalidOperationException(); + } +} + + namespace Diagnostics { #define DIAGNOSTIC(id, severity, name, messageFormat) const DiagnosticInfo name = { id, Severity::severity, messageFormat }; diff --git a/source/slang/diagnostics.h b/source/slang/diagnostics.h index 8834f1a6e..6957fc763 100644 --- a/source/slang/diagnostics.h +++ b/source/slang/diagnostics.h @@ -185,6 +185,12 @@ namespace Slang } void diagnoseImpl(CodePosition const& pos, DiagnosticInfo const& info, int argCount, DiagnosticArg const* const* args); + + // Add a diagnostic with raw text + // (used when we get errors from a downstream compiler) + void diagnoseRaw( + Severity severity, + char const* message); }; namespace Diagnostics -- cgit v1.2.3 From b65861fc32a54d28b3fbbaf0d31bb1d71889f570 Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Wed, 12 Jul 2017 13:32:24 -0700 Subject: Use C-style line directives, even for GLSL - As long as we are always going to pass GLSL through glslang, there should be no harm in this - Eventually we may need to re-enable the old style --- source/slang/emit.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'source') diff --git a/source/slang/emit.cpp b/source/slang/emit.cpp index 03cc7d8aa..3a5acd024 100644 --- a/source/slang/emit.cpp +++ b/source/slang/emit.cpp @@ -450,7 +450,17 @@ struct EmitVisitor emitRawText(" "); - if(context->shared->target == CodeGenTarget::GLSL) + bool shouldUseGLSLStyleLineDirective = false; + + // Let's not do this +#if 0 + if (context->shared->target == CodeGenTarget::GLSL) + { + shouldUseGLSLStyleLineDirective = true; + } +#endif + + if(shouldUseGLSLStyleLineDirective) { auto path = sourceLocation.FileName; -- cgit v1.2.3 From aca34e771d0ad76d396415e45c5f3cbf294709c9 Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Wed, 12 Jul 2017 13:40:30 -0700 Subject: Don't emit interpolation modifiers on struct fields when outputting GLSL HLSL (and thus Slang) commonly puts interpolation modifiers like `sample` on the fields of `struct` types used as stage input/output, while GLSL only allows them on global-scope `in` and `out` variables (or ones in blocks). This change emits a really hacky filtering step to skip over certain modifiers when emitting a declaration. This lets us skip interpolation-mode modifiers when outputting a struct field to GLSL. Note: this probably gets the `in` or `out` block case wrong... --- source/slang/emit.cpp | 32 ++++++++++++++++++++++++++++++++ source/slang/modifier-defs.h | 11 +++++++---- 2 files changed, 39 insertions(+), 4 deletions(-) (limited to 'source') diff --git a/source/slang/emit.cpp b/source/slang/emit.cpp index 3a5acd024..f5b4a8b56 100644 --- a/source/slang/emit.cpp +++ b/source/slang/emit.cpp @@ -2556,6 +2556,35 @@ struct EmitVisitor Emit(";\n"); } + bool shouldSkipModifierForDecl( + Modifier* modifier, + Decl* decl) + { + switch(context->shared->target) + { + default: + break; + + case CodeGenTarget::GLSL: + { + // Don't emit interpolation mode modifiers on `struct` fields + // (only allowed on global or block `in`/`out`) + if (auto interpolationMod = dynamic_cast(modifier)) + { + if (auto fieldDecl = dynamic_cast(decl)) + { + return true; + } + } + + } + break; + } + + + return false; + } + // Emit any modifiers that should go in front of a declaration void EmitModifiers(RefPtr decl) { @@ -2587,6 +2616,9 @@ struct EmitVisitor for (auto mod = decl->modifiers.first; mod; mod = mod->next) { + if (shouldSkipModifierForDecl(mod, decl)) + continue; + advanceToSourceLocation(mod->Position); if (0) {} diff --git a/source/slang/modifier-defs.h b/source/slang/modifier-defs.h index c401b3353..2b5c77d86 100644 --- a/source/slang/modifier-defs.h +++ b/source/slang/modifier-defs.h @@ -208,17 +208,20 @@ SIMPLE_SYNTAX_CLASS(GLSLColumnMajorLayoutModifier, RowMajorLayoutModifier) // More HLSL Keyword +ABSTRACT_SYNTAX_CLASS(InterpolationModeModifier, Modifier) +END_SYNTAX_CLASS() + // HLSL `nointerpolation` modifier -SIMPLE_SYNTAX_CLASS(HLSLNoInterpolationModifier, Modifier) +SIMPLE_SYNTAX_CLASS(HLSLNoInterpolationModifier, InterpolationModeModifier) // HLSL `linear` modifier -SIMPLE_SYNTAX_CLASS(HLSLLinearModifier, Modifier) +SIMPLE_SYNTAX_CLASS(HLSLLinearModifier, InterpolationModeModifier) // HLSL `sample` modifier -SIMPLE_SYNTAX_CLASS(HLSLSampleModifier, Modifier) +SIMPLE_SYNTAX_CLASS(HLSLSampleModifier, InterpolationModeModifier) // HLSL `centroid` modifier -SIMPLE_SYNTAX_CLASS(HLSLCentroidModifier, Modifier) +SIMPLE_SYNTAX_CLASS(HLSLCentroidModifier, InterpolationModeModifier) // HLSL `precise` modifier SIMPLE_SYNTAX_CLASS(HLSLPreciseModifier, Modifier) -- cgit v1.2.3 From d43ee03c6101ce76331135cebdc57711cb3a2020 Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Wed, 12 Jul 2017 13:48:20 -0700 Subject: Don't report error on assigning to an erroneous expression An expression with error type may still fail the l-value check, but we don't want to emit an error in that case. --- source/slang/check.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'source') diff --git a/source/slang/check.cpp b/source/slang/check.cpp index b87f7c6bc..b175b7f86 100644 --- a/source/slang/check.cpp +++ b/source/slang/check.cpp @@ -2540,7 +2540,11 @@ namespace Slang if (!type.IsLeftValue) { - if (!isRewriteMode()) + if (type->As()) + { + // Don't report an l-value issue on an errorneous expression + } + else if (!isRewriteMode()) { getSink()->diagnose(expr, Diagnostics::assignNonLValue); } -- cgit v1.2.3 From 02f77bbf12981abe376b2d5987684224a50ae4b2 Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Wed, 12 Jul 2017 14:21:52 -0700 Subject: Add ability for intrinsics to require GLSL extensions When cross-compiling, we need to detect when an intrinsic is used that required non-default GLSL capabilities and emit an appropriate `#extension ... : require` line. I'm handling this by attaching a custom modifier to declarations that require an extension in order to be callable. --- source/slang/emit.cpp | 30 ++++++++++++++++++++++++++++++ source/slang/modifier-defs.h | 6 ++++++ source/slang/parser.cpp | 11 +++++++++++ source/slang/slang-stdlib.cpp | 12 ++++++++++++ 4 files changed, 59 insertions(+) (limited to 'source') diff --git a/source/slang/emit.cpp b/source/slang/emit.cpp index f5b4a8b56..6905a8a76 100644 --- a/source/slang/emit.cpp +++ b/source/slang/emit.cpp @@ -52,6 +52,10 @@ struct SharedEmitContext ProgramSyntaxNode* program; bool needHackSamplerForTexelFetch = false; + + // Record the GLSL extnsions we have already emitted a `#extension` for + HashSet glslExtensionsRequired; + StringBuilder glslExtensionRequireLines; }; struct EmitContext @@ -1594,6 +1598,19 @@ struct EmitVisitor } } + void requireGLSLExtension(String const& name) + { + if (context->shared->glslExtensionsRequired.Contains(name)) + return; + + StringBuilder& sb = context->shared->glslExtensionRequireLines; + + sb.append("#extension "); + sb.append(name); + sb.append(" : require\n"); + + context->shared->glslExtensionsRequired.Add(name); + } void visitInvokeExpressionSyntaxNode( RefPtr callExpr, @@ -1708,6 +1725,17 @@ struct EmitVisitor } else if(auto targetIntrinsicModifier = findTargetIntrinsicModifier(funcDecl)) { + if (context->shared->target == CodeGenTarget::GLSL) + { + // Does this intrinsic requie a particular GLSL extension that wouldn't be available by default? + if (auto requiredGLSLExtensionModifier = funcDecl->FindModifier()) + { + // If so, we had better request the extension. + requireGLSLExtension(requiredGLSLExtensionModifier->extensionNameToken.Content); + } + } + + if(targetIntrinsicModifier->definitionToken.Type != TokenType::Unknown) { auto name = getStringOrIdentifierTokenValue(targetIntrinsicModifier->definitionToken); @@ -3550,6 +3578,8 @@ String emitEntryPoint( StringBuilder finalResultBuilder; finalResultBuilder << prefix; + finalResultBuilder << sharedContext.glslExtensionRequireLines.ProduceString(); + if (sharedContext.needHackSamplerForTexelFetch) { finalResultBuilder << "layout(set = 0, binding = 0) uniform sampler SLANG_hack_samplerForTexelFetch;\n"; diff --git a/source/slang/modifier-defs.h b/source/slang/modifier-defs.h index 2b5c77d86..9806a12a5 100644 --- a/source/slang/modifier-defs.h +++ b/source/slang/modifier-defs.h @@ -53,6 +53,12 @@ SYNTAX_CLASS(TargetIntrinsicModifier, IntrinsicModifierBase) FIELD(Token, definitionToken) END_SYNTAX_CLASS() +// A modifier to tag something as an intrinsic that requires +// a certain GLSL extension to be enabled when used +SYNTAX_CLASS(RequiredGLSLExtensionModifier, Modifier) + FIELD(Token, extensionNameToken) +END_SYNTAX_CLASS() + SIMPLE_SYNTAX_CLASS(InOutModifier, OutModifier) // This is a special sentinel modifier that gets added diff --git a/source/slang/parser.cpp b/source/slang/parser.cpp index de63793b1..8314b0930 100644 --- a/source/slang/parser.cpp +++ b/source/slang/parser.cpp @@ -717,6 +717,17 @@ namespace Slang AddModifier(&modifierLink, modifier); } + else if (AdvanceIf(parser, "__glsl_extension")) + { + auto modifier = new RequiredGLSLExtensionModifier(); + modifier->Position = loc; + + parser->ReadToken(TokenType::LParent); + modifier->extensionNameToken = parser->ReadToken(TokenType::Identifier); + parser->ReadToken(TokenType::RParent); + + AddModifier(&modifierLink, modifier); + } else if (AdvanceIf(parser, "layout")) { diff --git a/source/slang/slang-stdlib.cpp b/source/slang/slang-stdlib.cpp index bcc87b201..563f5a015 100644 --- a/source/slang/slang-stdlib.cpp +++ b/source/slang/slang-stdlib.cpp @@ -413,27 +413,33 @@ __intrinsic matrix ddx(matrix x); __generic +__glsl_extension(GL_ARB_derivative_control) __intrinsic(glsl, dFdxCoarse) __intrinsic T ddx_coarse(T x); __generic +__glsl_extension(GL_ARB_derivative_control) __intrinsic(glsl, dFdxCoarse) __intrinsic vector ddx_coarse(vector x); __generic +__glsl_extension(GL_ARB_derivative_control) __intrinsic(glsl, dFdxCoarse) __intrinsic matrix ddx_coarse(matrix x); __generic +__glsl_extension(GL_ARB_derivative_control) __intrinsic(glsl, dFdxFine) __intrinsic T ddx_fine(T x); __generic +__glsl_extension(GL_ARB_derivative_control) __intrinsic(glsl, dFdxFine) __intrinsic vector ddx_fine(vector x); __generic +__glsl_extension(GL_ARB_derivative_control) __intrinsic(glsl, dFdxFine) __intrinsic matrix ddx_fine(matrix x); @@ -452,27 +458,33 @@ __intrinsic matrix ddy(matrix x); __generic +__glsl_extension(GL_ARB_derivative_control) __intrinsic(glsl, dFdyCoarse) __intrinsic T ddy_coarse(T x); __generic +__glsl_extension(GL_ARB_derivative_control) __intrinsic(glsl, dFdyCoarse) __intrinsic vector ddy_coarse(vector x); __generic +__glsl_extension(GL_ARB_derivative_control) __intrinsic(glsl, dFdyCoarse) __intrinsic matrix ddy_coarse(matrix x); __generic +__glsl_extension(GL_ARB_derivative_control) __intrinsic(glsl, dFdyFine) __intrinsic T ddy_fine(T x); __generic +__glsl_extension(GL_ARB_derivative_control) __intrinsic(glsl, dFdyFine) __intrinsic vector ddy_fine(vector x); __generic +__glsl_extension(GL_ARB_derivative_control) __intrinsic(glsl, dFdyFine) __intrinsic matrix ddy_fine(matrix x); -- cgit v1.2.3