|
|
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.
|
