diff options
| author | Tim Foley <tfoleyNV@users.noreply.github.com> | 2018-06-14 07:29:45 -0700 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2018-06-14 07:29:45 -0700 |
| commit | 126e75d2266077752049140986c520158aa57361 (patch) | |
| tree | 61a57cbd80627b15f1a7adcc0eadd098c5194212 /source | |
| parent | 77562ef82bcbab569ebbbd769957948d825c92ad (diff) | |
Improve generic argument inference for builtins (#598)
Fixes #487
The basic problem here is that the user writes something like:
```hlsl
float invSqrt2 = 1 / sqrt(2);
```
In this case the user knows that `sqrt()` is only defined for floating-point types, so they expect this to compile something like:
```hlsl
float invSqrt2 = float(1) / sqrt(float(2));
```
The challenge this creates for the Slang compiler is that we use generics to streamline our declarations of all the builtins, so that the scalar `sqrt()` function is actually declared as:
```hlsl
T sqrt<T:__BuiltinFloatingPointType>(T value);
```
The `__BuiltinFloatingPointType` is an `interface` defined as part of the standard library, such that only built-in floating-point types conform to it (that is, `half`, `float`, and `double`).
When generic argument inference applies to a call like `sqrt(2)`, we see an argument of type `int`, and try to infer `T=int`, which leads to a failure because `int` does not conform to `__BuiltinFloatingPointType`.
The point where this currently fails in in the logic to "join" two types for inference, which is supposed to pick the best type that can represent both of two input types. E.g., a join between `float` and `int3` would be `float3`, since both of those types can convert to it, and it is the "minimal" type with that property.
So, the goal here is simple: we want a "join" between `int` and `__BuiltinFloatingPointType` to yield the `float` type. The way we handle that in this change is to special case the join of a basic scalar type and an interface, by enumerating all the basic scalar types, filtering them for ones that support the chosen interface and can be implicitly converted from the argument type, and then picking the "best" of them (the comments in the code explain what "best" means in this context).
The technique used here could be generalized in the future to deal with user-defined types or more cases, but that would risk slowing down overload resolution even more, which is already the most expensive part of our semantic checking pass.
A test case has been added for the specific case of `sqrt()` applied to an `int` argument.
Diffstat (limited to 'source')
| -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 |
3 files changed, 112 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 |
