Merge pull request #1089 from LoopDawg/split-shadow-tx

HLSL: split textures used for both shadow and non-shadow modes

Test/baseResults/hlsl.samplecmp.dualmode.frag.out

@@ -0,0 +1,157 @@
+hlsl.samplecmp.dualmode.frag
+WARNING: AST will form illegal SPIR-V; need to transform to legalize
+Shader version: 500
+gl_FragCoord origin is upper left
+0:? Sequence
+0:7  Function Definition: @main( ( temp 4-component vector of float)
+0:7    Function Parameters: 
+0:?     Sequence
+0:10      texture ( temp float)
+0:10        Construct combined texture-sampler ( temp sampler1DShadow)
+0:10          'g_tTex' (layout( binding=3) uniform texture1DShadow)
+0:10          'g_sSampCmp' (layout( binding=1) uniform sampler)
+0:10        Construct vec2 ( temp 2-component vector of float)
+0:10          Constant:
+0:10            0.100000
+0:10          Constant:
+0:10            0.750000
+0:11      texture ( temp 4-component vector of float)
+0:11        Construct combined texture-sampler ( temp sampler1D)
+0:11          'g_tTex' (layout( binding=3) uniform texture1D)
+0:11          'g_sSamp' (layout( binding=0) uniform sampler)
+0:11        Constant:
+0:11          0.100000
+0:13      Branch: Return with expression
+0:13        Constant:
+0:13          0.000000
+0:13          0.000000
+0:13          0.000000
+0:13          0.000000
+0:7  Function Definition: main( ( temp void)
+0:7    Function Parameters: 
+0:?     Sequence
+0:7      move second child to first child ( temp 4-component vector of float)
+0:?         '@entryPointOutput' (layout( location=0) out 4-component vector of float)
+0:7        Function Call: @main( ( temp 4-component vector of float)
+0:?   Linker Objects
+0:?     'g_sSamp' (layout( binding=0) uniform sampler)
+0:?     'g_sSampCmp' (layout( binding=1) uniform sampler)
+0:?     'g_tTex' (layout( binding=3) uniform texture1DShadow)
+0:?     '@entryPointOutput' (layout( location=0) out 4-component vector of float)
+0:?     'g_tTex' (layout( binding=3) uniform texture1D)
+
+
+Linked fragment stage:
+
+
+Shader version: 500
+gl_FragCoord origin is upper left
+0:? Sequence
+0:7  Function Definition: @main( ( temp 4-component vector of float)
+0:7    Function Parameters: 
+0:?     Sequence
+0:10      texture ( temp float)
+0:10        Construct combined texture-sampler ( temp sampler1DShadow)
+0:10          'g_tTex' (layout( binding=3) uniform texture1DShadow)
+0:10          'g_sSampCmp' (layout( binding=1) uniform sampler)
+0:10        Construct vec2 ( temp 2-component vector of float)
+0:10          Constant:
+0:10            0.100000
+0:10          Constant:
+0:10            0.750000
+0:11      texture ( temp 4-component vector of float)
+0:11        Construct combined texture-sampler ( temp sampler1D)
+0:11          'g_tTex' (layout( binding=3) uniform texture1D)
+0:11          'g_sSamp' (layout( binding=0) uniform sampler)
+0:11        Constant:
+0:11          0.100000
+0:13      Branch: Return with expression
+0:13        Constant:
+0:13          0.000000
+0:13          0.000000
+0:13          0.000000
+0:13          0.000000
+0:7  Function Definition: main( ( temp void)
+0:7    Function Parameters: 
+0:?     Sequence
+0:7      move second child to first child ( temp 4-component vector of float)
+0:?         '@entryPointOutput' (layout( location=0) out 4-component vector of float)
+0:7        Function Call: @main( ( temp 4-component vector of float)
+0:?   Linker Objects
+0:?     'g_sSamp' (layout( binding=0) uniform sampler)
+0:?     'g_sSampCmp' (layout( binding=1) uniform sampler)
+0:?     'g_tTex' (layout( binding=3) uniform texture1DShadow)
+0:?     '@entryPointOutput' (layout( location=0) out 4-component vector of float)
+0:?     'g_tTex' (layout( binding=3) uniform texture1D)
+
+// Module Version 10000
+// Generated by (magic number): 80001
+// Id's are bound by 43
+
+                              Capability Shader
+                              Capability Sampled1D
+               1:             ExtInstImport  "GLSL.std.450"
+                              MemoryModel Logical GLSL450
+                              EntryPoint Fragment 4  "main" 41
+                              ExecutionMode 4 OriginUpperLeft
+                              Source HLSL 500
+                              Name 4  "main"
+                              Name 9  "@main("
+                              Name 13  "g_tTex"
+                              Name 17  "g_sSampCmp"
+                              Name 29  "g_tTex"
+                              Name 31  "g_sSamp"
+                              Name 41  "@entryPointOutput"
+                              Decorate 13(g_tTex) DescriptorSet 0
+                              Decorate 13(g_tTex) Binding 3
+                              Decorate 17(g_sSampCmp) DescriptorSet 0
+                              Decorate 17(g_sSampCmp) Binding 1
+                              Decorate 29(g_tTex) DescriptorSet 0
+                              Decorate 29(g_tTex) Binding 3
+                              Decorate 31(g_sSamp) DescriptorSet 0
+                              Decorate 31(g_sSamp) Binding 0
+                              Decorate 41(@entryPointOutput) Location 0
+               2:             TypeVoid
+               3:             TypeFunction 2
+               6:             TypeFloat 32
+               7:             TypeVector 6(float) 4
+               8:             TypeFunction 7(fvec4)
+              11:             TypeImage 6(float) 1D depth sampled format:Unknown
+              12:             TypePointer UniformConstant 11
+      13(g_tTex):     12(ptr) Variable UniformConstant
+              15:             TypeSampler
+              16:             TypePointer UniformConstant 15
+  17(g_sSampCmp):     16(ptr) Variable UniformConstant
+              19:             TypeSampledImage 11
+              21:    6(float) Constant 1036831949
+              22:    6(float) Constant 1061158912
+              23:             TypeVector 6(float) 2
+              27:             TypeImage 6(float) 1D sampled format:Unknown
+              28:             TypePointer UniformConstant 27
+      29(g_tTex):     28(ptr) Variable UniformConstant
+     31(g_sSamp):     16(ptr) Variable UniformConstant
+              33:             TypeSampledImage 27
+              36:    6(float) Constant 0
+              37:    7(fvec4) ConstantComposite 36 36 36 36
+              40:             TypePointer Output 7(fvec4)
+41(@entryPointOutput):     40(ptr) Variable Output
+         4(main):           2 Function None 3
+               5:             Label
+              42:    7(fvec4) FunctionCall 9(@main()
+                              Store 41(@entryPointOutput) 42
+                              Return
+                              FunctionEnd
+       9(@main():    7(fvec4) Function None 8
+              10:             Label
+              14:          11 Load 13(g_tTex)
+              18:          15 Load 17(g_sSampCmp)
+              20:          19 SampledImage 14 18
+              24:   23(fvec2) CompositeConstruct 21 22
+              25:    6(float) CompositeExtract 24 1
+              26:    6(float) ImageSampleDrefImplicitLod 20 24 25
+              30:          27 Load 29(g_tTex)
+              32:          15 Load 31(g_sSamp)
+              34:          33 SampledImage 30 32
+              35:    7(fvec4) ImageSampleImplicitLod 34 21
+                              ReturnValue 37
+                              FunctionEnd

Test/hlsl.samplecmp.dualmode.frag

@@ -0,0 +1,14 @@
+SamplerState g_sSamp : register(s0);
+SamplerComparisonState g_sSampCmp : register(s1);
+
+uniform Texture1D <float4> g_tTex : register(t3);
+
+float4 main() : SV_Target0
+{
+    // This texture is used with both shadow modes.  It will need post-compilation
+    // legalization.
+    g_tTex.SampleCmp(g_sSampCmp, 0.1, 0.75);
+    g_tTex.Sample(g_sSamp, 0.1);
+
+    return 0;
+}

glslang/Include/intermediate.h

@@ -979,6 +979,7 @@ public:
         constSubtree(nullptr)
           { name = n; }
     virtual int getId() const { return id; }
+    virtual void setId(int newId) { id = newId; }
     virtual const TString& getName() const { return name; }
     virtual void traverse(TIntermTraverser*);
     virtual       TIntermSymbol* getAsSymbolNode()       { return this; }

gtests/Hlsl.FromFile.cpp

@@ -269,6 +269,7 @@ INSTANTIATE_TEST_CASE_P(
         {"hlsl.samplebias.offsetarray.dx10.frag", "main"},
         {"hlsl.samplecmp.array.dx10.frag", "main"},
         {"hlsl.samplecmp.basic.dx10.frag", "main"},
+        {"hlsl.samplecmp.dualmode.frag", "main"},
         {"hlsl.samplecmp.offset.dx10.frag", "main"},
         {"hlsl.samplecmp.offsetarray.dx10.frag", "main"},
         {"hlsl.samplecmp.negative.frag", "main"},

hlsl/hlslParseHelper.cpp

@@ -2936,37 +2936,66 @@ TIntermAggregate* HlslParseContext::handleSamplerTextureCombine(const TSourceLoc
 
     TSampler samplerType = argTex->getType().getSampler();
     samplerType.combined = true;
-    samplerType.shadow   = argSampler->getType().getSampler().shadow;
 
+    // TODO:
+    // This block exists until the spec no longer requires shadow modes on texture objects.
+    // It can be deleted after that, along with the shadowTextureVariant member.
     {
-        // ** TODO: **
+        const bool shadowMode = argSampler->getType().getSampler().shadow;
-        // This forces the texture's shadow state to be the sampler's
+
-        // shadow state.  This can't work if a single texture is used with
-        // both comparison and non-comparison samplers, so an error is
-        // reported if the shader does that.
-        //
-        // If this code is ever removed (possibly due to a relaxation in the
-        // SPIR-V rules), also remove the textureShadowMode member variable.
         TIntermSymbol* texSymbol = argTex->getAsSymbolNode();
 
         if (texSymbol == nullptr)
             texSymbol = argTex->getAsBinaryNode()->getLeft()->getAsSymbolNode();
 
-        if (texSymbol != nullptr) {
+        if (texSymbol == nullptr) {
-            const auto textureShadowModeEntry = textureShadowMode.find(texSymbol->getId());
+            error(loc, "unable to find texture symbol", "", "");
-
-            // Check to see if this texture has been given a different shadow mode already.
-            if (textureShadowModeEntry != textureShadowMode.end() &&
-                textureShadowModeEntry->second != samplerType.shadow) {
-                error(loc, "all uses of texture must use the same shadow mode", "", "");
             return nullptr;
         }
 
-            argTex->getWritableType().getSampler().shadow = samplerType.shadow;
+        // This forces the texture's shadow state to be the sampler's
-            textureShadowMode[texSymbol->getId()] = samplerType.shadow;
+        // shadow state.  This depends on downstream optimization to
-        }
+        // DCE one variant in [shadow, nonshadow] if both are present,
+        // or the SPIR-V module would be invalid.
+        int newId = texSymbol->getId();
+
+        // Check to see if this texture has been given a shadow mode already.
+        // If so, look up the one we already have.
+        const auto textureShadowEntry = textureShadowVariant.find(texSymbol->getId());
+
+        if (textureShadowEntry != textureShadowVariant.end())
+            newId = textureShadowEntry->second->get(shadowMode);
+        else
+            textureShadowVariant[texSymbol->getId()] = new tShadowTextureSymbols;
+
+        // Sometimes we have to create another symbol (if this texture has been seen before,
+        // and we haven't created the form for this shadow mode).
+        if (newId == -1) {
+            TType texType;
+            texType.shallowCopy(argTex->getType());
+            texType.getSampler().shadow = shadowMode;  // set appropriate shadow mode.
+            globalQualifierFix(loc, texType.getQualifier());
+
+            TVariable* newTexture = makeInternalVariable(texSymbol->getName(), texType);
+
+            trackLinkage(*newTexture);
+
+            newId = newTexture->getUniqueId();
         }
 
+        assert(newId != -1);
+
+        if (textureShadowVariant.find(newId) == textureShadowVariant.end())
+            textureShadowVariant[newId] = textureShadowVariant[texSymbol->getId()];
+
+        textureShadowVariant[newId]->set(shadowMode, newId);
+
+        // Remember this shadow mode in the texture and the merged type.
+        argTex->getWritableType().getSampler().shadow = shadowMode;
+        samplerType.shadow = shadowMode;
+
+        texSymbol->setId(newId);
+    }
 
     txcombine->setType(TType(samplerType, EvqTemporary));
     txcombine->setLoc(loc);
@@ -9506,13 +9535,18 @@ void HlslParseContext::fixTextureShadowModes()
         TSampler& sampler = (*symbol)->getWritableType().getSampler();
 
         if (sampler.isTexture()) {
-            const auto shadowMode = textureShadowMode.find((*symbol)->getUniqueId());
+            const auto shadowMode = textureShadowVariant.find((*symbol)->getUniqueId());
-            if (shadowMode != textureShadowMode.end())
+            if (shadowMode != textureShadowVariant.end()) {
-                sampler.shadow = shadowMode->second;
-        }
-    }
-}
 
+                if (shadowMode->second->overloaded())
+                    // Texture needs legalization if it's been seen with both shadow and non-shadow modes.
+                    intermediate.setNeedsLegalization();
+
+                sampler.shadow = shadowMode->second->isShadowId((*symbol)->getUniqueId());
+            }
+        }
+    }
+}
 
 // post-processing
 void HlslParseContext::finish()
@@ -9523,15 +9557,15 @@ void HlslParseContext::finish()
         error(mipsOperatorMipArg.back().loc, "unterminated mips operator:", "", "");
     }
 
+    removeUnusedStructBufferCounters();
+    addPatchConstantInvocation();
+    fixTextureShadowModes();
+
     // Communicate out (esp. for command line) that we formed AST that will make
     // illegal AST SPIR-V and it needs transforms to legalize it.
     if (intermediate.needsLegalization())
         infoSink.info << "WARNING: AST will form illegal SPIR-V; need to transform to legalize";
 
-    removeUnusedStructBufferCounters();
-    addPatchConstantInvocation();
-    fixTextureShadowModes();
-
     TParseContextBase::finish();
 }
 

hlsl/hlslParseHelper.h

@@ -457,10 +457,29 @@ protected:
 
     TVector<tMipsOperatorData> mipsOperatorMipArg;
 
-    // This can be removed if and when the texture shadow workarounnd in
+    // A texture object may be used with shadow and non-shadow samplers, but both may not be
-    // HlslParseContext::handleSamplerTextureCombine is removed.  It maps
+    // alive post-DCE in the same shader.  We do not know at compilation time which are alive: that's
-    // texture symbol IDs to the shadow modes of samplers they were combined with.
+    // only known post-DCE.  If a texture is used both ways, we create two textures, and
-    TMap<int, bool> textureShadowMode;
+    // leave the elimiation of one to the optimizer.  This maps the shader variant to
+    // the shadow variant.
+    //
+    // This can be removed if and when the texture shadow code in
+    // HlslParseContext::handleSamplerTextureCombine is removed.
+    struct tShadowTextureSymbols {
+        tShadowTextureSymbols() { symId.fill(-1); }
+
+        void set(bool shadow, int id) { symId[int(shadow)] = id; }
+        int get(bool shadow) const { return symId[int(shadow)]; }
+
+        // True if this texture has been seen with both shadow and non-shadow modes
+        bool overloaded() const { return symId[0] != -1 && symId[1] != -1; }
+        bool isShadowId(int id) const { return symId[1] == id; }
+
+    private:
+        std::array<int, 2> symId;
+    };
+
+    TMap<int, tShadowTextureSymbols*> textureShadowVariant;
 };
 
 // This is the prefix we use for built-in methods to avoid namespace collisions with