diff options
| author | Yong He <yonghe@outlook.com> | 2023-04-13 16:40:36 -0700 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2023-04-13 16:40:36 -0700 |
| commit | 3bbac5f16e9dd47acd2132c0bb2a43393831c450 (patch) | |
| tree | bddf137c252c164f7cd08f1f58b559b96ee49419 | |
| parent | 3b4a50d74059a26af2ed8c37fb2042f33ba7cf2c (diff) | |
Warn on float-to-double coercion for arguments. (#2802)
* Warn on float-to-double coercion for arguments.
* Fix test.
* Rename.
* Fixup.
---------
Co-authored-by: Yong He <yhe@nvidia.com>
| -rw-r--r-- | source/slang/core.meta.slang | 7 | ||||
| -rw-r--r-- | source/slang/slang-ast-modifier.h | 3 | ||||
| -rw-r--r-- | source/slang/slang-ast-support-types.h | 7 | ||||
| -rw-r--r-- | source/slang/slang-check-conversion.cpp | 32 | ||||
| -rw-r--r-- | source/slang/slang-check-decl.cpp | 12 | ||||
| -rw-r--r-- | source/slang/slang-check-expr.cpp | 8 | ||||
| -rw-r--r-- | source/slang/slang-check-impl.h | 14 | ||||
| -rw-r--r-- | source/slang/slang-check-modifier.cpp | 2 | ||||
| -rw-r--r-- | source/slang/slang-check-overload.cpp | 4 | ||||
| -rw-r--r-- | source/slang/slang-check-stmt.cpp | 4 | ||||
| -rw-r--r-- | source/slang/slang-diagnostic-defs.h | 1 | ||||
| -rw-r--r-- | source/slang/slang-parser.cpp | 7 | ||||
| -rw-r--r-- | tests/hlsl-intrinsic/matrix-double.slang | 2 |
13 files changed, 84 insertions, 19 deletions
diff --git a/source/slang/core.meta.slang b/source/slang/core.meta.slang index 9b2932446..3f1e99880 100644 --- a/source/slang/core.meta.slang +++ b/source/slang/core.meta.slang @@ -405,10 +405,15 @@ ${{{{ IROp intrinsicOpCode = getBaseTypeConversionOp( kBaseTypes[tt], kBaseTypes[ss]); + + BuiltinConversionKind builtinConversionKind = kBuiltinConversion_Unknown; + if (kBaseTypes[tt].tag == BaseType::Double && + kBaseTypes[ss].tag == BaseType::Float) + builtinConversionKind = kBuiltinConversion_FloatToDouble; }}}} __intrinsic_op($(intrinsicOpCode)) - __implicit_conversion($(conversionCost)) + __implicit_conversion($(conversionCost), $(builtinConversionKind)) __init($(kBaseTypes[ss].name) value); ${{{{ diff --git a/source/slang/slang-ast-modifier.h b/source/slang/slang-ast-modifier.h index 00a6570ef..0b077477d 100644 --- a/source/slang/slang-ast-modifier.h +++ b/source/slang/slang-ast-modifier.h @@ -956,6 +956,9 @@ class ImplicitConversionModifier : public Modifier // The conversion cost, used to rank conversions ConversionCost cost; + + // A builtin identifier for identifying conversions that need special treatment. + BuiltinConversionKind builtinConversionKind; }; class FormatAttribute : public Attribute diff --git a/source/slang/slang-ast-support-types.h b/source/slang/slang-ast-support-types.h index 78b0ccfcc..443b6e7bd 100644 --- a/source/slang/slang-ast-support-types.h +++ b/source/slang/slang-ast-support-types.h @@ -128,6 +128,13 @@ namespace Slang kConversionCost_Impossible = 0xFFFFFFFF, }; + typedef unsigned int BuiltinConversionKind; + enum : BuiltinConversionKind + { + kBuiltinConversion_Unknown = 0, + kBuiltinConversion_FloatToDouble = 1, + }; + enum class ImageFormat { #define FORMAT(NAME, OTHER) NAME, diff --git a/source/slang/slang-check-conversion.cpp b/source/slang/slang-check-conversion.cpp index abfe7afa0..7c4560b5b 100644 --- a/source/slang/slang-check-conversion.cpp +++ b/source/slang/slang-check-conversion.cpp @@ -23,6 +23,17 @@ namespace Slang return kConversionCost_Explicit; } + BuiltinConversionKind SemanticsVisitor::getImplicitConversionBuiltinKind( + Decl* decl) + { + if (auto modifier = decl->findModifier<ImplicitConversionModifier>()) + { + return modifier->builtinConversionKind; + } + + return kBuiltinConversion_Unknown; + } + bool SemanticsVisitor::isEffectivelyScalarForInitializerLists( Type* type) { @@ -121,6 +132,7 @@ namespace Slang { ioInitArgIndex++; return _coerce( + CoercionSite::Initializer, toType, outToExpr, firstInitExpr->type, @@ -211,6 +223,7 @@ namespace Slang { auto arg = fromInitializerListExpr->args[ioArgIndex++]; return _coerce( + CoercionSite::Initializer, toType, outToExpr, arg->type, @@ -616,6 +629,7 @@ namespace Slang } bool SemanticsVisitor::_coerce( + CoercionSite site, Type* toType, Expr** outToExpr, Type* fromType, @@ -852,6 +866,7 @@ namespace Slang } if(!_coerce( + site, toType, outToExpr, fromElementType, @@ -908,6 +923,7 @@ namespace Slang } if (!_coerce( + site, toType, outToExpr, fromValueType, @@ -1012,7 +1028,7 @@ namespace Slang // cost associated with the initializer we are invoking. // ConversionCost cost = getImplicitConversionCost( - overloadContext.bestCandidate->item.declRef.getDecl());; + overloadContext.bestCandidate->item.declRef.getDecl()); // If the cost is too high to be usable as an // implicit conversion, then we will report the @@ -1053,6 +1069,17 @@ namespace Slang getSink()->diagnose(fromExpr, Diagnostics::unrecommendedImplicitConversion, fromType, toType); } } + + if (site == CoercionSite::Argument) + { + auto builtinConversionKind = getImplicitConversionBuiltinKind( + overloadContext.bestCandidate->item.declRef.getDecl()); + if (builtinConversionKind == kBuiltinConversion_FloatToDouble) + { + if (!as<FloatingPointLiteralExpr>(fromExpr)) + getSink()->diagnose(fromExpr, Diagnostics::implicitConversionToDouble); + } + } } if(outCost) *outCost = cost; @@ -1146,6 +1173,7 @@ namespace Slang // during the coercion process. // bool rs = _coerce( + CoercionSite::General, toType, nullptr, fromType, @@ -1215,11 +1243,13 @@ namespace Slang Expr* SemanticsVisitor::coerce( + CoercionSite site, Type* toType, Expr* fromExpr) { Expr* expr = nullptr; if (!_coerce( + site, toType, &expr, fromExpr->type, diff --git a/source/slang/slang-check-decl.cpp b/source/slang/slang-check-decl.cpp index 63c7d9741..9b94ba492 100644 --- a/source/slang/slang-check-decl.cpp +++ b/source/slang/slang-check-decl.cpp @@ -1191,7 +1191,7 @@ namespace Slang if (auto initExpr = varDecl->initExpr) { initExpr = CheckTerm(initExpr); - initExpr = coerce(varDecl->type.Ptr(), initExpr); + initExpr = coerce(CoercionSite::Initializer, varDecl->type.Ptr(), initExpr); varDecl->initExpr = initExpr; maybeInferArraySizeForVariable(varDecl); @@ -1355,7 +1355,7 @@ namespace Slang // it to the type of the variable. // initExpr = CheckTerm(initExpr); - initExpr = coerce(varDecl->type.Ptr(), initExpr); + initExpr = coerce(CoercionSite::Initializer, varDecl->type.Ptr(), initExpr); varDecl->initExpr = initExpr; // We need to ensure that any variable doesn't introduce @@ -2860,7 +2860,7 @@ namespace Slang // so we also need to coerce the result of the call to // the expected type. // - auto coercedCall = subVisitor.coerce(resultType, checkedCall); + auto coercedCall = subVisitor.coerce(CoercionSite::Return, resultType, checkedCall); // If our overload resolution or type coercion failed, // then we have not been able to synthesize a witness @@ -3182,7 +3182,7 @@ namespace Slang // which involves coercing the member access `this.name` to // the expected type of the property. // - auto coercedMemberRef = subVisitor.coerce(propertyType, synMemberRef); + auto coercedMemberRef = subVisitor.coerce(CoercionSite::Return, propertyType, synMemberRef); auto synReturn = m_astBuilder->create<ReturnStmt>(); synReturn->expression = coercedMemberRef; @@ -4684,7 +4684,7 @@ namespace Slang if(auto initExpr = decl->tagExpr) { initExpr = CheckTerm(initExpr); - initExpr = coerce(tagType, initExpr); + initExpr = coerce(CoercionSite::General, tagType, initExpr); // We want to enforce that this is an integer constant // expression, but we don't actually care to retain @@ -5551,7 +5551,7 @@ namespace Slang // actual type of the parameter. // initExpr = CheckTerm(initExpr); - initExpr = coerce(typeExpr.type, initExpr); + initExpr = coerce(CoercionSite::Initializer, typeExpr.type, initExpr); paramDecl->initExpr = initExpr; // TODO: a default argument expression needs to diff --git a/source/slang/slang-check-expr.cpp b/source/slang/slang-check-expr.cpp index 0f744997d..507b8c30f 100644 --- a/source/slang/slang-check-expr.cpp +++ b/source/slang/slang-check-expr.cpp @@ -1618,13 +1618,13 @@ namespace Slang switch (coercionType) { case IntegerConstantExpressionCoercionType::SpecificType: - expr = coerce(expectedType, inExpr); + expr = coerce(CoercionSite::General, expectedType, inExpr); break; case IntegerConstantExpressionCoercionType::AnyInteger: if (isScalarIntegerType(inExpr->type)) expr = inExpr; else - expr = coerce(m_astBuilder->getIntType(), inExpr); + expr = coerce(CoercionSite::General, m_astBuilder->getIntType(), inExpr); break; default: break; @@ -1862,7 +1862,7 @@ namespace Slang expr->left = maybeOpenRef(expr->left); auto type = expr->left->type; auto right = maybeOpenRef(expr->right); - expr->right = coerce(type, right); + expr->right = coerce(CoercionSite::Assignment, type, right); if (!type.isLeftValue) { @@ -2650,7 +2650,7 @@ namespace Slang initListExpr->loc = expr->loc; auto checkedInitListExpr = visitInitializerListExpr(initListExpr); - return coerce(typeExp.type, checkedInitListExpr); + return coerce(CoercionSite::General, typeExp.type, checkedInitListExpr); } } } diff --git a/source/slang/slang-check-impl.h b/source/slang/slang-check-impl.h index 50992b00b..18a60b8b3 100644 --- a/source/slang/slang-check-impl.h +++ b/source/slang/slang-check-impl.h @@ -219,6 +219,15 @@ namespace Slang Dictionary<BasicTypeKeyPair, ConversionCost> conversionCostCache; }; + enum class CoercionSite + { + General, + Assignment, + Argument, + Return, + Initializer, + }; + /// Shared state for a semantics-checking session. struct SharedSemanticsContext { @@ -773,6 +782,9 @@ namespace Slang ConversionCost getImplicitConversionCost( Decl* decl); + BuiltinConversionKind getImplicitConversionBuiltinKind( + Decl* decl); + bool isEffectivelyScalarForInitializerLists( Type* type); @@ -886,6 +898,7 @@ namespace Slang /// should be emitted on failure. /// bool _coerce( + CoercionSite site, Type* toType, Expr** outToExpr, Type* fromType, @@ -924,6 +937,7 @@ namespace Slang /// Implicitly coerce `fromExpr` to `toType` and diagnose errors if it isn't possible Expr* coerce( + CoercionSite site, Type* toType, Expr* fromExpr); diff --git a/source/slang/slang-check-modifier.cpp b/source/slang/slang-check-modifier.cpp index a068f19d6..d96d9e39e 100644 --- a/source/slang/slang-check-modifier.cpp +++ b/source/slang/slang-check-modifier.cpp @@ -559,7 +559,7 @@ namespace Slang if (!typeChecked) { arg = CheckTerm(arg); - arg = coerce(paramDecl->getType(), arg); + arg = coerce(CoercionSite::Argument, paramDecl->getType(), arg); } } paramIndex++; diff --git a/source/slang/slang-check-overload.cpp b/source/slang/slang-check-overload.cpp index f786089f8..d544ce5a1 100644 --- a/source/slang/slang-check-overload.cpp +++ b/source/slang/slang-check-overload.cpp @@ -320,7 +320,7 @@ namespace Slang } else { - arg = coerce(getType(m_astBuilder, valParamRef), arg); + arg = coerce(CoercionSite::Argument, getType(m_astBuilder, valParamRef), arg); } // If we have an argument to work with, then we will @@ -427,7 +427,7 @@ namespace Slang } else { - arg = coerce(paramType, arg); + arg = coerce(CoercionSite::Argument, paramType, arg); } } return true; diff --git a/source/slang/slang-check-stmt.cpp b/source/slang/slang-check-stmt.cpp index 6efc3e52e..b2830febe 100644 --- a/source/slang/slang-check-stmt.cpp +++ b/source/slang/slang-check-stmt.cpp @@ -157,7 +157,7 @@ namespace Slang } Expr* e = expr; e = CheckTerm(e); - e = coerce(m_astBuilder->getBoolType(), e); + e = coerce(CoercionSite::General, m_astBuilder->getBoolType(), e); return e; } @@ -313,7 +313,7 @@ namespace Slang { if (function) { - stmt->expression = coerce(function->returnType.Ptr(), stmt->expression); + stmt->expression = coerce(CoercionSite::Return, function->returnType.Ptr(), stmt->expression); } else { diff --git a/source/slang/slang-diagnostic-defs.h b/source/slang/slang-diagnostic-defs.h index 128142d84..dcd376474 100644 --- a/source/slang/slang-diagnostic-defs.h +++ b/source/slang/slang-diagnostic-defs.h @@ -302,6 +302,7 @@ DIAGNOSTIC(33071, Error, expectedAStringLiteral, "expected a string literal") DIAGNOSTIC( -1, Note, noteExplicitConversionPossible, "explicit conversion from '$0' to '$1' is possible") DIAGNOSTIC(30080, Error, ambiguousConversion, "more than one implicit conversion exists from '$0' to '$1'") DIAGNOSTIC(30081, Warning, unrecommendedImplicitConversion, "implicit conversion from '$0' to '$1' is not recommended") +DIAGNOSTIC(30082, Warning, implicitConversionToDouble, " implicit float-to-double conversion may cause unexpected performance issues, use explicit cast if intended.") DIAGNOSTIC(30090, Error, tryClauseMustApplyToInvokeExpr, "expression in a 'try' clause must be a call to a function or operator overload.") DIAGNOSTIC(30091, Error, tryInvokeCalleeShouldThrow, "'$0' called from a 'try' clause does not throw an error, make sure the callee is marked as 'throws'") DIAGNOSTIC(30092, Error, calleeOfTryCallMustBeFunc, "callee in a 'try' clause must be a function") diff --git a/source/slang/slang-parser.cpp b/source/slang/slang-parser.cpp index 45f4be477..555c79445 100644 --- a/source/slang/slang-parser.cpp +++ b/source/slang/slang-parser.cpp @@ -6560,14 +6560,19 @@ namespace Slang static NodeBase* parseImplicitConversionModifier(Parser* parser, void* /*userData*/) { ImplicitConversionModifier* modifier = parser->astBuilder->create<ImplicitConversionModifier>(); - + BuiltinConversionKind builtinKind = kBuiltinConversion_Unknown; ConversionCost cost = kConversionCost_Default; if( AdvanceIf(parser, TokenType::LParent) ) { cost = ConversionCost(StringToInt(parser->ReadToken(TokenType::IntegerLiteral).getContent())); + if (AdvanceIf(parser, TokenType::Comma)) + { + builtinKind = BuiltinConversionKind(StringToInt(parser->ReadToken(TokenType::IntegerLiteral).getContent())); + } parser->ReadToken(TokenType::RParent); } modifier->cost = cost; + modifier->builtinConversionKind = builtinKind; return modifier; } diff --git a/tests/hlsl-intrinsic/matrix-double.slang b/tests/hlsl-intrinsic/matrix-double.slang index 08bd78cee..31ec54f8e 100644 --- a/tests/hlsl-intrinsic/matrix-double.slang +++ b/tests/hlsl-intrinsic/matrix-double.slang @@ -101,7 +101,7 @@ void test1(inout FloatMatrix ft, inout FloatMatrix f, int idx) { float scalarVs[] = { 1, 10, 100, 1000 }; - ft += FloatMatrix(IntMatrix(log10(makeFloatMatrix(scalarVs[idx])) + makeFloatMatrix(0.5f))); + ft += FloatMatrix(IntMatrix(log10(makeFloatMatrix(Float(scalarVs[idx]))) + makeFloatMatrix(0.5f))); } ft += abs(f * makeFloatMatrix(4) - makeFloatMatrix(2.0f)); |
