summaryrefslogtreecommitdiffstats
path: root/source/slang/ir.h
diff options
context:
space:
mode:
authorTim Foley <tfoleyNV@users.noreply.github.com>2019-03-12 10:45:39 -0700
committerGitHub <noreply@github.com>2019-03-12 10:45:39 -0700
commit9722990745f4e91ab1bf9fb682c72e173cd123b4 (patch)
tree730eeda0208b6bb72a74cb5e3cfac22fe77fe91f /source/slang/ir.h
parent35c26f3395383b49031b672b77470062d74a5f59 (diff)
Fix handling of arrays of resources in type legalization (#896)
Before type legalization we might have code like: (using pseudo-Slang-IR): struct P { ... Texture2D<float>[] t; } global_param p : ParameterBlock<P>; ... // p.t[someIndex].Load(...); // let ptrToArrayOfTextures = getFieldPtr(p, "t") : Ptr<Texture2D<float>[]>; let ptrToTexture = getElementPtr(ptrToArrayOfTextures, someIndex) : Ptr<Texture2D<float>>; let texture = load(ptrToTexture) : Texture2D<float>; let result = call(loadFunc, texture, ...) : float; Legalization needs to move the `t` array there out of the `p` parameter block, so the global declarations become something like: struct P_Ordinary { ... }; // no more "t" field global_param p_ordinary : ParameterBlock<p_ordinary); global_param p_t : Texture2D<float>[]; In terms of the code to access `p.t[someIndex]` the problem is that `p_t` has one less level of indirection than `p.t` had. We solve this in the type legalization pass using "pseudo-types" and "pseudo-values," where one of the cases is `implicitIndirect` which holds a value of type `T`, but indicates that it should act like a value of type `T*`. We then use some basic rules for dealing with `implicitIndirect` values, such as: load(implicitDeref(x)) : T => x : T getFieldPtr(implicitDeref(s), f) => implicitDeref(getField(s, f)) getElementPtr(implicitDeref(a), i) => implicitDeref(getElement(a, i)) The bug here was that for the `getFieldPtr` and `getElementPtr` cases, we weren't computing the type of the `getField` or `getElement` instruction correctly. We were copying the type from the `getFieldPtr` or `getElementPtr` operation over directly, but those will be *pointer* types and we need the type of whatever they point to. Once the types are fixed, we can properly generate legalized IR for `p.t[someIndex].Load(...) that looks like: let arrayOfTextures = p_t : Texture2D<float>[]; let texture = getElement(arrayOfTextures, someIndex) : Texture2D<float>; let result = call(loadFunc, texture, ...) : float; The old was giving the `texture` intermediate a type of `Ptr<Texure2D<float>>`. That didn't actually trip up too many things, because we mostly just went on to emit code from something with slightly incorrect types for intermediates that never show up in the generated HLSL/GLSL. Where this caused a problem is for some of the intrinsic function definitions for the GLSL/Vulkan back-end, because those do things that inspect operand types. In particular the `$z` opcode in our intrinsic function strings triggers logic that looks at a texture operand, and uses its type to try to find the appropriate swizzle to get from a 4-component vector to the appropriate type for the operation (e.g., for a load from a `Texture2D<float>` we need to swizzle with `.x` to get a single scalar out of the matching GLSL texture fetch operation). The main fix in this change is thus to make `getElementPtr` and `getFieldPtr` legalization properly account for the fact that when switching to `getElement` or `getField` we need a result type that is the "pointee" of the original result. There was already logic to extract the pointed-to type from a pointer in `ir-specialize.cpp`, so I extracted that to a re-usable function in the IR as `tryGetPointedToType` (returns null if the type isn't actually a pointer). This logic needed to be extended for type legalization, to deal with the various "pseudo-type" cases. There is another fix in this change which is marking the `NonUniformResourceIndex` function as `[__readNone]`, which enables it to be more aggressively folded into use sites. Without that fix, we risk emitting code like: ```glsl int tmp = nonUniformEXT(someIndex); vec4 result = texelFetch(arrayOfTextures[tmp], ...); ``` The problem with that code is that (at least by my reading of the spec), assigning to the variable `tmp` that isn't declared with the `nonUniformEXT` qualifier effectively loses that qualifier, and drivers are free to assume that `tmp` is uniform when used to index into `arrayOfTextures`. Marking the `NonUniformResourceIndex` function as `[__readNone]` indicates that it has no side effects, which should mean that our emit logic no longer needs to emit it was its own line of code to be safe. The effects of this change are confirmed by both the new test case added, and the existing `non-uniform-indexing` test.
Diffstat (limited to 'source/slang/ir.h')
-rw-r--r--source/slang/ir.h8
1 files changed, 8 insertions, 0 deletions
diff --git a/source/slang/ir.h b/source/slang/ir.h
index 09013fe48..d4497ec91 100644
--- a/source/slang/ir.h
+++ b/source/slang/ir.h
@@ -25,6 +25,7 @@ class Layout;
class Type;
class Session;
class Name;
+struct IRBuilder;
struct IRFunc;
struct IRGlobalValueWithCode;
struct IRInst;
@@ -934,6 +935,13 @@ SIMPLE_IR_TYPE(OutType, OutTypeBase)
SIMPLE_IR_TYPE(InOutType, OutTypeBase)
SIMPLE_IR_TYPE(ExistentialBoxType, PtrTypeBase)
+ /// Get the type pointed to be `ptrType`, or `nullptr` if it is not a pointer(-like) type.
+ ///
+ /// The given IR `builder` will be used if new instructions need to be created.
+IRType* tryGetPointedToType(
+ IRBuilder* builder,
+ IRType* type);
+
struct IRFuncType : IRType
{
IRType* getResultType() { return (IRType*) getOperand(0); }