From d84cfb766b089c03f9545588a8590efea1749770 Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Mon, 15 Jun 2020 12:05:04 -0700 Subject: Remove implicit conversions to `void` (#1388) * Remove implicit conversions to `void` Fixes #1372 The standard library code had accidentally introduced implicit-conversion `__init` operations on the `void` type that accepted each of the other basic types, so that a function written like: ```hlsl void bad() { return 1; } ``` would translate to: ```hlsl void bad() { return (void)1; } ``` The dual problesm are that the input code should have produced a diagnostic of some kind, and the output code doesn't appear to compile correctly through fxc. This change introduces several fixes aimed at this issue: * First, the problem in the stdlib code is plugged: we don't introduce implicit conversion operations *to* or *from* `void` (we'd only been banning it in one direction before) * Next, an explicit `__init` was added to `void` that accepts *any* type so that existing HLSL code that might do `(void) someExpression` to ignore a result will continue to work. This is a compatibility feature, and it might be argued that we should at least warn when it is used. Note that this function is expected to never appear in output HLSL/GLSL because its result will never be used, and it is marked `[__readNone]` allowing calls to it to be eliminated as dead code. * During IR lowering, we now take care to only emit the `IRReturnVal` instruction type if there is a non-`void` value being returned, and use `IRReturnVoid` for both the case where no expression was used in the `return` statement *and* the case where an expression of type `void` is returned. * A test case was added to confirm that returning `1` from a `void` function isn't allowed, while returning `(void) 1` *is*. The net result of these changes is that we now produce an error for the bad input code, we allow explicit casts to `void` as a compatibility feature, and we are more robust about treating `void` as if it is an ordinary type in the front-end. * fixup: missing file --- source/slang/core.meta.slang | 19 ++++++++++++++++++- source/slang/slang-lower-to-ir.cpp | 39 +++++++++++++++++++++++++++++++++----- 2 files changed, 52 insertions(+), 6 deletions(-) (limited to 'source') diff --git a/source/slang/core.meta.slang b/source/slang/core.meta.slang index 0c39f2c2f..b56ca3085 100644 --- a/source/slang/core.meta.slang +++ b/source/slang/core.meta.slang @@ -192,7 +192,9 @@ ${{{{ // Declare initializers to convert from various other types for (int ss = 0; ss < kBaseTypeCount; ++ss) { - // Don't allow conversion from `void` + // Don't allow conversion to or from `void` + if (kBaseTypes[tt].tag == BaseType::Void) + continue; if (kBaseTypes[ss].tag == BaseType::Void) continue; @@ -250,6 +252,21 @@ ${{{{ ${{{{ break; } + + // If this is the `void` type, then we want to allow + // explicit conversion to it from any other type, using + // `(void) someExpression`. + // + if( kBaseTypes[tt].tag == BaseType::Void ) + { +}}}} + __generic + [__readNone] + __init(T value) + {} +${{{{ + } + }}}} } diff --git a/source/slang/slang-lower-to-ir.cpp b/source/slang/slang-lower-to-ir.cpp index 42f53ef22..fa23b3307 100644 --- a/source/slang/slang-lower-to-ir.cpp +++ b/source/slang/slang-lower-to-ir.cpp @@ -3610,18 +3610,47 @@ struct StmtLoweringVisitor : StmtVisitor { startBlockIfNeeded(stmt); - // A `return` statement turns into a return - // instruction. If the statement had an argument - // expression, then we need to lower that to - // a value first, and then emit the resulting value. + // A `return` statement turns into a `return` instruction, + // but we have two kinds of `return`: one for returning + // a (non-`void`) value, and one for returning "no value" + // (which effectively returns a value of type `void`). + // if( auto expr = stmt->expression ) { + // If the AST `return` statement had an expression, then we + // need to lower it to the IR at this point, both to + // compute its value and (in case we are returning a + // `void`-typed expression) to execute its side effects. + // auto loweredExpr = lowerRValueExpr(context, expr); - getBuilder()->emitReturn(getSimpleVal(context, loweredExpr)); + // If the AST `return` statement was returning a non-`void` + // value, then we need to emit an IR `return` of that value. + // + if(!expr->type.type->equals(context->astBuilder->getVoidType())) + { + getBuilder()->emitReturn(getSimpleVal(context, loweredExpr)); + } + else + { + // If the type of the value returned was `void`, then + // we don't want to emit an IR-level `return` with a value, + // because that could trip up some of our back-end. + // + // TODO: We should eventually have only a single IR-level + // `return` operation that always takes a value (including + // values of type `void`), and then treat an AST `return;` + // as equivalent to something like `return void();`. + // + getBuilder()->emitReturn(); + } } else { + // If we hit this case, then the AST `return` was a `return;` + // with no value, which can only occur in a function with + // a `void` result type. + // getBuilder()->emitReturn(); } } -- cgit v1.2.3 From 3461ed41118480c3494f810cddd3cd8c735e2fbb Mon Sep 17 00:00:00 2001 From: Yong He Date: Mon, 15 Jun 2020 13:55:56 -0700 Subject: Specialize function calls involving array arguments. (#1389) Fixes #890. Co-authored-by: Tim Foley --- source/slang/slang-ir-specialize-resources.cpp | 34 +++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) (limited to 'source') diff --git a/source/slang/slang-ir-specialize-resources.cpp b/source/slang/slang-ir-specialize-resources.cpp index f72ca6b38..60439abeb 100644 --- a/source/slang/slang-ir-specialize-resources.cpp +++ b/source/slang/slang-ir-specialize-resources.cpp @@ -171,6 +171,27 @@ struct ResourceParameterSpecializationContext return anySpecializableParam; } + // Returns true if `type` is an `IRStructType` with array-typed fields. + bool isStructTypeWithArray(IRType* type) + { + if (auto structType = as(type)) + { + for (auto field : structType->getFields()) + { + if (auto arrayType = as(field->getFieldType())) + { + return true; + } + if (auto subStructType = as(field->getFieldType())) + { + if (isStructTypeWithArray(subStructType)) + return true; + } + } + } + return false; + } + // Of course, now we need to back-fill the predicates that // the above function used to evaluate prameters and arguments. @@ -209,6 +230,8 @@ struct ResourceParameterSpecializationContext return true; if(as(type)) return true; + if (isStructTypeWithArray(type)) + return true; } // For now, we will not treat any other parameters as @@ -257,7 +280,7 @@ struct ResourceParameterSpecializationContext // of the indexing operation is also // suitable for specialization. // - if( arg->op == kIROp_getElement ) + if( arg->op == kIROp_getElement || arg->op == kIROp_Load ) { auto base = arg->getOperand(0); @@ -565,6 +588,11 @@ struct ResourceParameterSpecializationContext // ioInfo.newArgs.add(oldIndex); } + else if (oldArg->op == kIROp_Load) + { + auto oldBase = oldArg->getOperand(0); + getCallInfoForArg(ioInfo, oldBase); + } else { // If we fail to match any of the cases above @@ -718,6 +746,10 @@ struct ResourceParameterSpecializationContext return newVal; } + else if (oldArg->op == kIROp_Load) + { + return getSpecializedValueForArg(ioInfo, oldArg->getOperand(0)); + } else { // If we don't match one of the above cases, -- cgit v1.2.3