Merge pull request #812 from steve-lunarg/PatchIO

Test/hlsl.hull.ctrlpt-2.tesc

+// *** 
+// per-control-point invocation of PCF from entry point return value with
+// both OutputPatch and InputPatch given to PCF.
+// ***
+
+struct hs_in_t
+{
+    float3 val : TEXCOORD0;
+};
+
+struct hs_pcf_t
+{
+    float tfactor[3] : SV_TessFactor; // must turn into a size 4 array in SPIR-V
+    float flInFactor : SV_InsideTessFactor; // must turn into a size 2 array in SPIR-V
+}; 
+
+struct hs_out_t
+{
+    float3 val : TEXCOORD0; 
+};
+
+[ domain ("tri") ]
+[ partitioning ("fractional_odd") ]
+[ outputtopology ("triangle_cw") ]
+[ outputcontrolpoints (3) ]
+[ patchconstantfunc ( "PCF" ) ]
+hs_out_t main (InputPatch <hs_in_t, 3> i , uint cpid : SV_OutputControlPointID)
+{
+    i[0].val;
+
+    hs_out_t o;
+    o.val = cpid;
+    return o;
+}
+
+hs_pcf_t PCF( const OutputPatch <hs_out_t, 3> pcf_out,
+              const InputPatch <hs_in_t, 3> pcf_in)
+{
+    hs_pcf_t o;
+
+    o.tfactor[0] = pcf_out[0].val.x;
+    o.tfactor[1] = pcf_out[1].val.x;
+    o.tfactor[2] = pcf_out[2].val.x;
+    o.flInFactor = 4;
+
+    return o;
+}

glslang/Include/BaseTypes.h

@@ -220,6 +220,8 @@ enum TBuiltInVariable {
     EbvFragDepthLesser,
     EbvStencilRef,
     EbvGsOutputStream,
+    EbvOutputPatch,
+    EbvInputPatch,
 
     EbvLast
 };

gtests/Hlsl.FromFile.cpp

@@ -129,6 +129,7 @@ INSTANTIATE_TEST_CASE_P(
         {"hlsl.hull.2.tesc", "main"},
         {"hlsl.hull.void.tesc", "main"},
         {"hlsl.hull.ctrlpt-1.tesc", "main"},
+        {"hlsl.hull.ctrlpt-2.tesc", "main"},
         {"hlsl.identifier.sample.frag", "main"},
         {"hlsl.if.frag", "PixelShaderFunction"},
         {"hlsl.implicitBool.frag", "main"},

hlsl/hlslGrammar.cpp

@@ -963,14 +963,14 @@ bool HlslGrammar::acceptOutputPrimitiveGeometry(TLayoutGeometry& geometry)
 //      : INPUTPATCH
 //      | OUTPUTPATCH
 //
-bool HlslGrammar::acceptTessellationDeclType()
+bool HlslGrammar::acceptTessellationDeclType(TBuiltInVariable& patchType)
 {
     // read geometry type
     const EHlslTokenClass tessType = peek();
 
     switch (tessType) {
-    case EHTokInputPatch:    break;
-    case EHTokOutputPatch:   break;
+    case EHTokInputPatch:    patchType = EbvInputPatch;  break;
+    case EHTokOutputPatch:   patchType = EbvOutputPatch; break;
     default:
         return false;  // not a tessellation decl
     }
@@ -984,7 +984,9 @@ bool HlslGrammar::acceptTessellationDeclType()
 //
 bool HlslGrammar::acceptTessellationPatchTemplateType(TType& type)
 {
-    if (! acceptTessellationDeclType())
+    TBuiltInVariable patchType;
+
+    if (! acceptTessellationDeclType(patchType))
         return false;
     
     if (! acceptTokenClass(EHTokLeftAngle))
@@ -1011,6 +1013,7 @@ bool HlslGrammar::acceptTessellationPatchTemplateType(TType& type)
     TArraySizes* arraySizes = new TArraySizes;
     arraySizes->addInnerSize(size->getAsConstantUnion()->getConstArray()[0].getIConst());
     type.newArraySizes(*arraySizes);
+    type.getQualifier().builtIn = patchType;
 
     if (! acceptTokenClass(EHTokRightAngle)) {
         expected("right angle bracket");

hlsl/hlslGrammar.h

@@ -79,7 +79,7 @@ namespace glslang {
         bool acceptTemplateVecMatBasicType(TBasicType&);
         bool acceptVectorTemplateType(TType&);
         bool acceptMatrixTemplateType(TType&);
-        bool acceptTessellationDeclType();
+        bool acceptTessellationDeclType(TBuiltInVariable&);
         bool acceptTessellationPatchTemplateType(TType&);
         bool acceptStreamOutTemplateType(TType&, TLayoutGeometry&);
         bool acceptOutputPrimitiveGeometry(TLayoutGeometry&);

hlsl/hlslParseHelper.cpp

@@ -67,7 +67,8 @@ HlslParseContext::HlslParseContext(TSymbolTable& symbolTable, TIntermediate& int
     sourceEntryPointName(sourceEntryPointName),
     entryPointFunction(nullptr),
     entryPointFunctionBody(nullptr),
-    gsStreamOutput(nullptr)
+    gsStreamOutput(nullptr),
+    inputPatch(nullptr)
 {
     globalUniformDefaults.clear();
     globalUniformDefaults.layoutMatrix = ElmRowMajor;
@@ -1377,6 +1378,7 @@ TIntermTyped* HlslParseContext::splitAccessStruct(const TSourceLoc& loc, TInterm
 void HlslParseContext::trackLinkage(TSymbol& symbol)
 {
     TBuiltInVariable biType = symbol.getType().getQualifier().builtIn;
+
     if (biType != EbvNone)
         builtInLinkageSymbols[biType] = symbol.clone();
 
@@ -1811,6 +1813,7 @@ TIntermNode* HlslParseContext::transformEntryPoint(const TSourceLoc& loc, TFunct
             else if (variable.getType().containsBuiltInInterstageIO(language))
                 split(variable);
         }
+
         assignLocations(variable);
     };
     if (entryPointOutput)
@@ -1988,6 +1991,7 @@ void HlslParseContext::remapEntryPointIO(TFunction& function, TVariable*& return
             correctOutput(ioVariable->getWritableType().getQualifier());
         }
         ioVariable->getWritableType().getQualifier().storage = storage;
+
         return ioVariable;
     };
 
@@ -2022,6 +2026,9 @@ void HlslParseContext::remapEntryPointIO(TFunction& function, TVariable*& return
         if (paramType.getQualifier().isParamInput()) {
             TVariable* argAsGlobal = makeIoVariable(function[i].name->c_str(), paramType, EvqVaryingIn);
             inputs.push_back(argAsGlobal);
+
+            if (function[i].declaredBuiltIn == EbvInputPatch)
+                inputPatch = argAsGlobal;
         }
         if (paramType.getQualifier().isParamOutput()) {
             TVariable* argAsGlobal = makeIoVariable(function[i].name->c_str(), paramType, EvqVaryingOut);
@@ -7574,7 +7581,10 @@ void HlslParseContext::addPatchConstantInvocation()
     // Look for builtin variables in a function's parameter list.
     const auto findBuiltIns = [&](const TFunction& function, std::set<tInterstageIoData>& builtIns) {
         for (int p=0; p<function.getParamCount(); ++p) {
-            const TStorageQualifier storage = function[p].type->getQualifier().storage;
+            TStorageQualifier storage = function[p].type->getQualifier().storage;
+
+            if (storage == EvqConstReadOnly) // treated identically to input
+                storage = EvqIn;
 
             if (function[p].declaredBuiltIn != EbvNone)
                 builtIns.insert(HlslParseContext::tInterstageIoData(function[p].declaredBuiltIn, storage));
@@ -7605,9 +7615,11 @@ void HlslParseContext::addPatchConstantInvocation()
         }
     };
 
-    const auto isPerCtrlPt = [this](const TType& type) {
-        // TODO: this is not sufficient to reject all such cases in malformed shaders.
-        return type.isArray() && !type.isRuntimeSizedArray();
+    const auto isOutputPatch = [this](TFunction& patchConstantFunction, int param) {
+        const TType& type = *patchConstantFunction[param].type;
+        const TBuiltInVariable biType = patchConstantFunction[param].declaredBuiltIn;
+
+        return type.isArray() && !type.isRuntimeSizedArray() && biType == EbvOutputPatch;
     };
     
     // We will perform these steps.  Each is in a scoped block for separation: they could
@@ -7636,7 +7648,7 @@ void HlslParseContext::addPatchConstantInvocation()
     TIntermSymbol* invocationIdSym = findLinkageSymbol(EbvInvocationId);
     TIntermSequence& epBodySeq = entryPointFunctionBody->getAsAggregate()->getSequence();
 
-    int perCtrlPtParam = -1; // -1 means there isn't one.
+    int outPatchParam = -1; // -1 means there isn't one.
 
     // ================ Step 1A: Union Interfaces ================
     // Our patch constant function.
@@ -7662,20 +7674,31 @@ void HlslParseContext::addPatchConstantInvocation()
         // Now we'll add those to the entry and to the linkage.
         for (int p=0; p<pcfParamCount; ++p) {
             const TBuiltInVariable biType   = patchConstantFunction[p].declaredBuiltIn;
-            const TStorageQualifier storage = patchConstantFunction[p].type->getQualifier().storage;
+            TStorageQualifier storage = patchConstantFunction[p].type->getQualifier().storage;
 
-            // Track whether there is any per control point input
-            if (isPerCtrlPt(*patchConstantFunction[p].type)) {
-                if (perCtrlPtParam >= 0) {
-                    // Presently we only support one per ctrl pt input. TODO: does HLSL even allow multiple?
-                    error(loc, "unimplemented: multiple per control point inputs to patch constant function", "", "");
+            // Track whether there is an output patch param
+            if (isOutputPatch(patchConstantFunction, p)) {
+                if (outPatchParam >= 0) {
+                    // Presently we only support one per ctrl pt input.
+                    error(loc, "unimplemented: multiple output patches in patch constant function", "", "");
                     return;
                 }
-                perCtrlPtParam = p;
+                outPatchParam = p;
             }
 
             if (biType != EbvNone) {
                 TType* paramType = patchConstantFunction[p].type->clone();
+
+                if (storage == EvqConstReadOnly) // treated identically to input
+                    storage = EvqIn;
+
+                // Presently, the only non-builtin we support is InputPatch, which is treated as
+                // a pseudo-builtin.
+                if (biType == EbvInputPatch) {
+                    builtInLinkageSymbols[biType] = inputPatch;
+                } else if (biType == EbvOutputPatch) {
+                    // Nothing...
+                } else {
                     // Use the original declaration type for the linkage
                     paramType->getQualifier().builtIn = biType;
 
@@ -7683,6 +7706,7 @@ void HlslParseContext::addPatchConstantInvocation()
                         addToLinkage(*paramType, patchConstantFunction[p].name, nullptr);
                 }
             }
+        }
 
         // If we didn't find it because the shader made one, add our own.
         if (invocationIdSym == nullptr) {
@@ -7703,18 +7727,12 @@ void HlslParseContext::addPatchConstantInvocation()
     // TODO: handle struct or array inputs
     {
         for (int p=0; p<pcfParamCount; ++p) {
-            if ((patchConstantFunction[p].type->isArray() && !isPerCtrlPt(*patchConstantFunction[p].type)) ||
-                (!patchConstantFunction[p].type->isArray() && patchConstantFunction[p].type->isStruct())) {
-                error(loc, "unimplemented array or variable in patch constant function signature", "", "");
-                return;
-            }
-        
             TIntermSymbol* inputArg = nullptr;
 
-            if (p == perCtrlPtParam) {
+            if (p == outPatchParam) {
                 if (perCtrlPtVar == nullptr) {
-                    perCtrlPtVar = makeInternalVariable(*patchConstantFunction[perCtrlPtParam].name,
-                                                        *patchConstantFunction[perCtrlPtParam].type);
+                    perCtrlPtVar = makeInternalVariable(*patchConstantFunction[outPatchParam].name,
+                                                        *patchConstantFunction[outPatchParam].type);
 
                     perCtrlPtVar->getWritableType().getQualifier().makeTemporary();
                 }
@@ -7769,9 +7787,9 @@ void HlslParseContext::addPatchConstantInvocation()
     // invocations of the entry point to build up an array, or (TODO:) use a yet
     // unavailable extension to look across the SIMD lanes.  This is the former
     // as a placeholder for the latter.
-    if (perCtrlPtParam >= 0) {
+    if (outPatchParam >= 0) {
         // We must introduce a local temp variable of the type wanted by the PCF input.
-        const int arraySize = patchConstantFunction[perCtrlPtParam].type->getOuterArraySize();
+        const int arraySize = patchConstantFunction[outPatchParam].type->getOuterArraySize();
 
         if (entryPointFunction->getType().getBasicType() == EbtVoid) {
             error(loc, "entry point must return a value for use with patch constant function", "", "");

hlsl/hlslParseHelper.h

@@ -386,6 +386,7 @@ protected:
     };
 
     TMap<tInterstageIoData, TVariable*> interstageBuiltInIo; // individual builtin interstage IO vars, indexed by builtin type.
+    TVariable* inputPatch;
 
     // We have to move array references to structs containing builtin interstage IO to the split variables.
     // This is only handled for one level.  This stores the index, because we'll need it in the future, since