Parameters of spirv_decorate_id should accept variables

spirv_decorate_id will generate OpDecorateId. The parameter list should
accept variables as part of decorations. This is because OpDecorateId
allows this. The spec says:

  All such <id> Extra Operands must be constant instructions or
  OpVariable instructions.

SPIRV/GlslangToSpv.cpp

@@ -9449,10 +9449,10 @@ spv::Id TGlslangToSpvTraverser::getSymbolId(const glslang::TIntermSymbol* symbol
             std::vector<spv::Id> operandIds;
             assert(!decorateId.second.empty());
             for (auto extraOperand : decorateId.second) {
-                if (extraOperand->getQualifier().isSpecConstant())
+                if (extraOperand->getQualifier().isFrontEndConstant())
-                    operandIds.push_back(getSymbolId(extraOperand->getAsSymbolNode()));
-                else
                     operandIds.push_back(createSpvConstant(*extraOperand));
+                else
+                    operandIds.push_back(getSymbolId(extraOperand->getAsSymbolNode()));
             }
             builder.addDecorationId(id, static_cast<spv::Decoration>(decorateId.first), operandIds);
         }

Test/baseResults/spv.intrinsicsSpirvDecorateId.comp.out

@@ -0,0 +1,46 @@
+spv.intrinsicsSpirvDecorateId.comp
+// Module Version 10000
+// Generated by (magic number): 8000b
+// Id's are bound by 16
+
+                              Capability Shader
+                              Extension  "SPV_GOOGLE_hlsl_functionality1"
+               1:             ExtInstImport  "GLSL.std.450"
+                              MemoryModel Logical GLSL450
+                              EntryPoint GLCompute 4  "main"
+                              ExecutionMode 4 LocalSize 1 1 1
+                              Source GLSL 460
+                              SourceExtension  "GL_EXT_spirv_intrinsics"
+                              Name 4  "main"
+                              Name 10  "CounterBuffer"
+                              MemberName 10(CounterBuffer) 0  "counter"
+                              Name 12  "x"
+                              Name 13  "Uniform"
+                              MemberName 13(Uniform) 0  "y"
+                              Name 15  ""
+                              Decorate 9 BuiltIn WorkgroupSize
+                              MemberDecorate 10(CounterBuffer) 0 Offset 0
+                              Decorate 10(CounterBuffer) Block
+                              Decorate 12(x) DescriptorSet 0
+                              Decorate 12(x) Binding 1
+                              MemberDecorate 13(Uniform) 0 Offset 0
+                              Decorate 13(Uniform) Block
+                              Decorate 15 DescriptorSet 0
+                              Decorate 15 Binding 0
+                              DecorateId 15 DecorationHlslCounterBufferGOOGLE 12(x)
+               2:             TypeVoid
+               3:             TypeFunction 2
+               6:             TypeInt 32 0
+               7:             TypeVector 6(int) 3
+               8:      6(int) Constant 1
+               9:    7(ivec3) ConstantComposite 8 8 8
+10(CounterBuffer):             TypeStruct 6(int)
+              11:             TypePointer Uniform 10(CounterBuffer)
+           12(x):     11(ptr) Variable Uniform
+     13(Uniform):             TypeStruct 6(int)
+              14:             TypePointer Uniform 13(Uniform)
+              15:     14(ptr) Variable Uniform
+         4(main):           2 Function None 3
+               5:             Label
+                              Return
+                              FunctionEnd

Test/spv.intrinsicsSpirvDecorateId.comp

@@ -0,0 +1,16 @@
+#version 460 core
+#extension GL_EXT_spirv_intrinsics: enable
+
+layout(local_size_x = 1) in;
+
+layout(binding = 1) uniform CounterBuffer {
+  uint counter;
+} x;
+
+layout(binding = 0) spirv_decorate_id(extensions = ["SPV_GOOGLE_hlsl_functionality1"], 5634, x) uniform Uniform {
+  uint y;
+};
+
+void main()
+{
+}

glslang/MachineIndependent/SpirvIntrinsics.cpp

@@ -168,7 +168,7 @@ void TQualifier::setSpirvDecorateId(int decoration, const TIntermAggregate* args
     TVector<const TIntermTyped*> extraOperands;
     for (auto arg : args->getSequence()) {
         auto extraOperand = arg->getAsTyped();
-        assert(extraOperand != nullptr && extraOperand->getQualifier().isConstant());
+        assert(extraOperand != nullptr);
         extraOperands.push_back(extraOperand);
     }
     spirvDecorate->decorateIds[decoration] = extraOperands;
@@ -202,30 +202,29 @@ TString TQualifier::getSpirvDecorateQualifierString() const
     const auto appendStr = [&](const char* s) { qualifierString.append(s); };
 
     const auto appendDecorate = [&](const TIntermTyped* constant) {
-        auto& constArray = constant->getAsConstantUnion() != nullptr ? constant->getAsConstantUnion()->getConstArray()
+        if (constant->getAsConstantUnion()) {
-                                                                     : constant->getAsSymbolNode()->getConstArray();
+            auto& constArray = constant->getAsConstantUnion()->getConstArray();
-        if (constant->getBasicType() == EbtFloat) {
+            if (constant->getBasicType() == EbtFloat) {
-            float value = static_cast<float>(constArray[0].getDConst());
+                float value = static_cast<float>(constArray[0].getDConst());
-            appendFloat(value);
+                appendFloat(value);
+            } else if (constant->getBasicType() == EbtInt) {
+                int value = constArray[0].getIConst();
+                appendInt(value);
+            } else if (constant->getBasicType() == EbtUint) {
+                unsigned value = constArray[0].getUConst();
+                appendUint(value);
+            } else if (constant->getBasicType() == EbtBool) {
+                bool value = constArray[0].getBConst();
+                appendBool(value);
+            } else if (constant->getBasicType() == EbtString) {
+                const TString* value = constArray[0].getSConst();
+                appendStr(value->c_str());
+            } else
+                assert(0);
+        } else {
+            assert(constant->getAsSymbolNode());
+            appendStr(constant->getAsSymbolNode()->getName().c_str());
         }
-        else if (constant->getBasicType() == EbtInt) {
-            int value = constArray[0].getIConst();
-            appendInt(value);
-        }
-        else if (constant->getBasicType() == EbtUint) {
-            unsigned value = constArray[0].getUConst();
-            appendUint(value);
-        }
-        else if (constant->getBasicType() == EbtBool) {
-            bool value = constArray[0].getBConst();
-            appendBool(value);
-        }
-        else if (constant->getBasicType() == EbtString) {
-            const TString* value = constArray[0].getSConst();
-            appendStr(value->c_str());
-        }
-        else
-            assert(0);
     };
 
     for (auto& decorate : spirvDecorate->decorates) {

glslang/MachineIndependent/glslang.m4

@@ -378,7 +378,7 @@ GLSLANG_WEB_EXCLUDE_ON
 %type <interm.type> spirv_storage_class_qualifier
 %type <interm.type> spirv_decorate_qualifier
 %type <interm.intermNode> spirv_decorate_parameter_list spirv_decorate_parameter
-%type <interm.intermNode> spirv_decorate_id_parameter_list
+%type <interm.intermNode> spirv_decorate_id_parameter_list spirv_decorate_id_parameter
 %type <interm.intermNode> spirv_decorate_string_parameter_list
 %type <interm.type> spirv_type_specifier
 %type <interm.spirvTypeParams> spirv_type_parameter_list spirv_type_parameter
@@ -4347,23 +4347,33 @@ spirv_decorate_parameter
     }
 
 spirv_decorate_id_parameter_list
-    : constant_expression {
+    : spirv_decorate_id_parameter {
-        if ($1->getBasicType() != EbtFloat &&
-            $1->getBasicType() != EbtInt &&
-            $1->getBasicType() != EbtUint &&
-            $1->getBasicType() != EbtBool)
-            parseContext.error($1->getLoc(), "this type not allowed", $1->getType().getBasicString(), "");
         $$ = parseContext.intermediate.makeAggregate($1);
     }
-    | spirv_decorate_id_parameter_list COMMA constant_expression {
+    | spirv_decorate_id_parameter_list COMMA spirv_decorate_id_parameter {
-        if ($3->getBasicType() != EbtFloat &&
-            $3->getBasicType() != EbtInt &&
-            $3->getBasicType() != EbtUint &&
-            $3->getBasicType() != EbtBool)
-            parseContext.error($3->getLoc(), "this type not allowed", $3->getType().getBasicString(), "");
         $$ = parseContext.intermediate.growAggregate($1, $3);
     }
 
+spirv_decorate_id_parameter
+    : variable_identifier {
+        if ($1->getAsConstantUnion() || $1->getAsSymbolNode())
+            $$ = $1;
+        else
+            parseContext.error($1->getLoc(), "only allow constants or variables which are not elements of a composite", "", "");
+    }
+    | FLOATCONSTANT {
+        $$ = parseContext.intermediate.addConstantUnion($1.d, EbtFloat, $1.loc, true);
+    }
+    | INTCONSTANT {
+        $$ = parseContext.intermediate.addConstantUnion($1.i, $1.loc, true);
+    }
+    | UINTCONSTANT {
+        $$ = parseContext.intermediate.addConstantUnion($1.u, $1.loc, true);
+    }
+    | BOOLCONSTANT {
+        $$ = parseContext.intermediate.addConstantUnion($1.b, $1.loc, true);
+    }
+
 spirv_decorate_string_parameter_list
     : STRING_LITERAL {
         $$ = parseContext.intermediate.makeAggregate(

glslang/MachineIndependent/glslang.y

@@ -378,7 +378,7 @@ extern int yylex(YYSTYPE*, TParseContext&);
 %type <interm.type> spirv_storage_class_qualifier
 %type <interm.type> spirv_decorate_qualifier
 %type <interm.intermNode> spirv_decorate_parameter_list spirv_decorate_parameter
-%type <interm.intermNode> spirv_decorate_id_parameter_list
+%type <interm.intermNode> spirv_decorate_id_parameter_list spirv_decorate_id_parameter
 %type <interm.intermNode> spirv_decorate_string_parameter_list
 %type <interm.type> spirv_type_specifier
 %type <interm.spirvTypeParams> spirv_type_parameter_list spirv_type_parameter
@@ -4347,23 +4347,33 @@ spirv_decorate_parameter
     }
 
 spirv_decorate_id_parameter_list
-    : constant_expression {
+    : spirv_decorate_id_parameter {
-        if ($1->getBasicType() != EbtFloat &&
-            $1->getBasicType() != EbtInt &&
-            $1->getBasicType() != EbtUint &&
-            $1->getBasicType() != EbtBool)
-            parseContext.error($1->getLoc(), "this type not allowed", $1->getType().getBasicString(), "");
         $$ = parseContext.intermediate.makeAggregate($1);
     }
-    | spirv_decorate_id_parameter_list COMMA constant_expression {
+    | spirv_decorate_id_parameter_list COMMA spirv_decorate_id_parameter {
-        if ($3->getBasicType() != EbtFloat &&
-            $3->getBasicType() != EbtInt &&
-            $3->getBasicType() != EbtUint &&
-            $3->getBasicType() != EbtBool)
-            parseContext.error($3->getLoc(), "this type not allowed", $3->getType().getBasicString(), "");
         $$ = parseContext.intermediate.growAggregate($1, $3);
     }
 
+spirv_decorate_id_parameter
+    : variable_identifier {
+        if ($1->getAsConstantUnion() || $1->getAsSymbolNode())
+            $$ = $1;
+        else
+            parseContext.error($1->getLoc(), "only allow constants or variables which are not elements of a composite", "", "");
+    }
+    | FLOATCONSTANT {
+        $$ = parseContext.intermediate.addConstantUnion($1.d, EbtFloat, $1.loc, true);
+    }
+    | INTCONSTANT {
+        $$ = parseContext.intermediate.addConstantUnion($1.i, $1.loc, true);
+    }
+    | UINTCONSTANT {
+        $$ = parseContext.intermediate.addConstantUnion($1.u, $1.loc, true);
+    }
+    | BOOLCONSTANT {
+        $$ = parseContext.intermediate.addConstantUnion($1.b, $1.loc, true);
+    }
+
 spirv_decorate_string_parameter_list
     : STRING_LITERAL {
         $$ = parseContext.intermediate.makeAggregate(

gtests/Spv.FromFile.cpp

@@ -384,6 +384,7 @@ INSTANTIATE_TEST_SUITE_P(
         "spv.intOps.vert",
         "spv.intrinsicsSpirvByReference.vert",
         "spv.intrinsicsSpirvDecorate.frag",
+        "spv.intrinsicsSpirvDecorateId.comp",
         "spv.intrinsicsSpirvExecutionMode.frag",
         "spv.intrinsicsSpirvInstruction.vert",
         "spv.intrinsicsSpirvLiteral.vert",