diff options
| -rw-r--r-- | source/slang/check.cpp | 114 | ||||
| -rw-r--r-- | source/slang/core.meta.slang.h | 2 | ||||
| -rw-r--r-- | source/slang/type-system-shared.h | 6 | ||||
| -rw-r--r-- | tests/bugs/gh-487.slang | 30 | ||||
| -rw-r--r-- | tests/bugs/gh-487.slang.expected.txt | 4 |
5 files changed, 146 insertions, 10 deletions
diff --git a/source/slang/check.cpp b/source/slang/check.cpp index 764764e76..046587d0f 100644 --- a/source/slang/check.cpp +++ b/source/slang/check.cpp @@ -5117,21 +5117,119 @@ namespace Slang return result; } + /// Does there exist an implicit conversion from `fromType` to `toType`? + bool canConvertImplicitly( + RefPtr<Type> toType, + RefPtr<Type> fromType) + { + // Can we convert at all? + ConversionCost conversionCost; + if(!CanCoerce(toType, fromType, &conversionCost)) + return false; + + // Is the conversion cheap enough to be done implicitly? + if(conversionCost >= kConversionCost_GeneralConversion) + return false; + + return true; + } + RefPtr<Type> TryJoinTypeWithInterface( - RefPtr<Type> type, - DeclRef<InterfaceDecl> interfaceDeclRef) + RefPtr<Type> type, + DeclRef<InterfaceDecl> interfaceDeclRef) { // The most basic test here should be: does the type declare conformance to the trait. if(DoesTypeConformToInterface(type, interfaceDeclRef)) return type; - // There is a more nuanced case if `type` is a builtin type, and we need to make it - // conform to a trait that some but not all builtin types support (the main problem - // here is when an operation wants an integer type, but one of our operands is a `float`. - // The HLSL rules will allow that, with implicit conversion, but our default join rules - // will end up picking `float` and we don't want that...). + // Just because `type` doesn't conform to the given `interfaceDeclRef`, that + // doesn't necessarily indicate a failure. It is possible that we have a call + // like `sqrt(2)` so that `type` is `int` and `interfaceDeclRef` is + // `__BuiltinFloatingPointType`. The "obvious" answer is that we should infer + // the type `float`, but it seems like the compiler would have to synthesize + // that answer from thin air. + // + // A robsut/correct solution here might be to enumerate set of types types `S` + // such that for each type `X` in `S`: + // + // * `type` is implicitly convertible to `X` + // * `X` conforms to the interface named by `interfaceDeclRef` + // + // If the set `S` is non-empty then we would try to pick the "best" type from `S`. + // The "best" type would be a type `Y` such that `Y` is implicitly convertible to + // every other type in `S`. + // + // We are going to implement a much simpler strategy for now, where we only apply + // the search process if `type` is a builtin scalar type, and then we only search + // through types `X` that are also builtin scalar types. + // + RefPtr<Type> bestType; + if(auto basicType = type.As<BasicExpressionType>()) + { + basicType->baseType; + for(Int baseTypeFlavorIndex = 0; baseTypeFlavorIndex < Int(BaseType::CountOf); baseTypeFlavorIndex++) + { + // Don't consider `type`, since we already know it doesn't work. + if(baseTypeFlavorIndex == Int(basicType->baseType)) + continue; + + // Look up the type in our session. + auto candidateType = type->getSession()->getBuiltinType(BaseType(baseTypeFlavorIndex)); + if(!candidateType) + continue; + + // We only want to consider types that implement the target interface. + if(!DoesTypeConformToInterface(candidateType, interfaceDeclRef)) + continue; + + // We only want to consider types where we can implicitly convert from `type` + if(!canConvertImplicitly(candidateType, type)) + continue; + + // At this point, we have a candidate type that is usable. + // + // If this is our first viable candidate, then it is our best one: + // + if(!bestType) + { + bestType = candidateType; + } + else + { + // Otherwise, we want to pick the "better" type between `candidateType` + // and `bestType`. + // + // We are going to be a bit loose here, and not worry about the + // case where conversion is allowed in both directions. + // + // TODO: make this completely robust. + // + if(canConvertImplicitly(bestType, candidateType)) + { + // Our candidate can convert to the current "best" type, so + // it is logically a more specific type that satisfies our + // constraints, thereforce we should keep it. + // + bestType = candidateType; + } + } + } + if(bestType) + return bestType; + } + + // For all other cases, we will just bail out for now. + // + // TODO: In the future we should build some kind of side data structure + // to accelerate either one or both of these queries: + // + // * Given a type `T`, what types `U` can it convert to implicitly? + // + // * Given an interface `I`, what types `U` conform to it? + // + // The intersection of the sets returned by these two queries is + // the set of candidates we would like to consider here. - // For now we don't handle the hard case and just bail return nullptr; } diff --git a/source/slang/core.meta.slang.h b/source/slang/core.meta.slang.h index bdd618976..b4bef2397 100644 --- a/source/slang/core.meta.slang.h +++ b/source/slang/core.meta.slang.h @@ -66,7 +66,9 @@ for (int tt = 0; tt < kBaseTypeCount; ++tt) switch (kBaseTypes[tt].tag) { + case BaseType::Half: case BaseType::Float: + case BaseType::Double: sb << "\n , __BuiltinFloatingPointType\n"; sb << "\n , __BuiltinRealType\n"; // fall through to: diff --git a/source/slang/type-system-shared.h b/source/slang/type-system-shared.h index 61e0ebac7..dd06026c5 100644 --- a/source/slang/type-system-shared.h +++ b/source/slang/type-system-shared.h @@ -21,6 +21,8 @@ namespace Slang #define DEFINE_BASE_TYPE(NAME) NAME, FOREACH_BASE_TYPE(DEFINE_BASE_TYPE) #undef DEFINE_BASE_TYPE + + CountOf, }; struct TextureFlavor @@ -39,7 +41,7 @@ FOREACH_BASE_TYPE(DEFINE_BASE_TYPE) // Whether or not this is a shadow texture // // TODO(tfoley): is this even meaningful/used? - // ShadowFlag = 0x80, + // ShadowFlag = 0x80, }; enum Shape : uint8_t @@ -88,4 +90,4 @@ FOREACH_BASE_TYPE(DEFINE_BASE_TYPE) } -#endif
\ No newline at end of file +#endif diff --git a/tests/bugs/gh-487.slang b/tests/bugs/gh-487.slang new file mode 100644 index 000000000..ee01ee9b1 --- /dev/null +++ b/tests/bugs/gh-487.slang @@ -0,0 +1,30 @@ +// gh-487.slang +//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute + +// This test is to confirm that we can apply builtin functions taht expect +// a floating-point argument to an integer, with the compiler filling +// in the implicit conversion. This is made tricky by the fact +// that a builtin line `sqrt` is actually a constrained generic, +// `sqrt<T:BuiltinFloatingPointType>` so that inference currently +// fails to deduce `T=float` when presented with an `int` argument. + +int test(int val) +{ + int squared = val * val; + float result = sqrt(squared); + return int(result); +} + +//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):dxbinding(0),glbinding(0),out +RWStructuredBuffer<int> gBuffer; + +[numthreads(4, 1, 1)] +void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID) +{ + uint tid = dispatchThreadID.x; + + int inVal = tid; + int outVal = test(inVal); + + gBuffer[tid] = outVal; +}
\ No newline at end of file diff --git a/tests/bugs/gh-487.slang.expected.txt b/tests/bugs/gh-487.slang.expected.txt new file mode 100644 index 000000000..bc856dafa --- /dev/null +++ b/tests/bugs/gh-487.slang.expected.txt @@ -0,0 +1,4 @@ +0 +1 +2 +3 |