Update priv-to-local for SPIR-V 1.4 (#2567)

Fixes #2555

* Fix a bug in validation where interfaces were considered non-unique
between different entry points targeting the same function
  * added a test
* Update private to local pass to remove localized private variables
from entry point interfaces
  * added tests

source/opt/private_to_local_pass.cpp

@@ -19,6 +19,7 @@
 #include <vector>
 
 #include "source/opt/ir_context.h"
+#include "source/spirv_constant.h"
 
 namespace spvtools {
 namespace opt {
@@ -38,6 +39,7 @@ Pass::Status PrivateToLocalPass::Process() {
     return Status::SuccessWithoutChange;
 
   std::vector<std::pair<Instruction*, Function*>> variables_to_move;
+  std::unordered_set<uint32_t> localized_variables;
   for (auto& inst : context()->types_values()) {
     if (inst.opcode() != SpvOpVariable) {
       continue;
@@ -57,6 +59,27 @@ Pass::Status PrivateToLocalPass::Process() {
   modified = !variables_to_move.empty();
   for (auto p : variables_to_move) {
     MoveVariable(p.first, p.second);
+    localized_variables.insert(p.first->result_id());
+  }
+
+  if (get_module()->version() >= SPV_SPIRV_VERSION_WORD(1, 4)) {
+    // In SPIR-V 1.4 and later entry points must list private storage class
+    // variables that are statically used by the entry point. Go through the
+    // entry points and remove any references to variables that were localized.
+    for (auto& entry : get_module()->entry_points()) {
+      std::vector<Operand> new_operands;
+      for (uint32_t i = 0; i < entry.NumInOperands(); ++i) {
+        // Execution model, function id and name are always kept.
+        if (i < 3 ||
+            !localized_variables.count(entry.GetSingleWordInOperand(i))) {
+          new_operands.push_back(entry.GetInOperand(i));
+        }
+      }
+      if (new_operands.size() != entry.NumInOperands()) {
+        entry.SetInOperands(std::move(new_operands));
+        context()->AnalyzeUses(&entry);
+      }
+    }
   }
 
   return (modified ? Status::SuccessWithChange : Status::SuccessWithoutChange);
@@ -165,6 +188,7 @@ void PrivateToLocalPass::UpdateUse(Instruction* inst) {
       UpdateUses(inst->result_id());
       break;
     case SpvOpName:
+    case SpvOpEntryPoint:  // entry points will be updated separately.
       break;
     default:
       assert(spvOpcodeIsDecoration(inst->opcode()) &&

source/val/validate_decorations.cpp

@@ -683,8 +683,8 @@ spv_result_t CheckDecorationsOfEntryPoints(ValidationState_t& vstate) {
     const auto& descs = vstate.entry_point_descriptions(entry_point);
     int num_builtin_inputs = 0;
     int num_builtin_outputs = 0;
-    std::unordered_set<Instruction*> seen_vars;
     for (const auto& desc : descs) {
+      std::unordered_set<Instruction*> seen_vars;
       for (auto interface : desc.interfaces) {
         Instruction* var_instr = vstate.FindDef(interface);
         if (!var_instr || SpvOpVariable != var_instr->opcode()) {

test/opt/private_to_local_test.cpp

@@ -308,6 +308,117 @@ TEST_F(PrivateToLocalTest, CreatePointerToAmbiguousStruct2) {
   SinglePassRunAndMatch<PrivateToLocalPass>(text, false);
 }
 
+TEST_F(PrivateToLocalTest, SPV14RemoveFromInterface) {
+  const std::string text = R"(
+; CHECK-NOT: OpEntryPoint GLCompute %foo "foo" %in %priv
+; CHECK: OpEntryPoint GLCompute %foo "foo" %in
+; CHECK: %priv = OpVariable {{%\w+}} Function
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint GLCompute %foo "foo" %in %priv
+OpExecutionMode %foo LocalSize 1 1 1
+OpName %foo "foo"
+OpName %in "in"
+OpName %priv "priv"
+%void = OpTypeVoid
+%int = OpTypeInt 32 0
+%ptr_ssbo_int = OpTypePointer StorageBuffer %int
+%ptr_private_int = OpTypePointer Private %int
+%in = OpVariable %ptr_ssbo_int StorageBuffer
+%priv = OpVariable %ptr_private_int Private
+%void_fn = OpTypeFunction %void
+%foo = OpFunction %void None %void_fn
+%entry = OpLabel
+%ld = OpLoad %int %in
+OpStore %priv %ld
+OpReturn
+OpFunctionEnd
+)";
+
+  SetTargetEnv(SPV_ENV_UNIVERSAL_1_4);
+  SinglePassRunAndMatch<PrivateToLocalPass>(text, true);
+}
+
+TEST_F(PrivateToLocalTest, SPV14RemoveFromInterfaceMultipleEntryPoints) {
+  const std::string text = R"(
+; CHECK-NOT: OpEntryPoint GLCompute %foo "foo" %in %priv
+; CHECK-NOT: OpEntryPoint GLCompute %foo "bar" %in %priv
+; CHECK: OpEntryPoint GLCompute %foo "foo" %in
+; CHECK: OpEntryPoint GLCompute %foo "bar" %in
+; CHECK: %priv = OpVariable {{%\w+}} Function
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint GLCompute %foo "foo" %in %priv
+OpEntryPoint GLCompute %foo "bar" %in %priv
+OpExecutionMode %foo LocalSize 1 1 1
+OpName %foo "foo"
+OpName %in "in"
+OpName %priv "priv"
+%void = OpTypeVoid
+%int = OpTypeInt 32 0
+%ptr_ssbo_int = OpTypePointer StorageBuffer %int
+%ptr_private_int = OpTypePointer Private %int
+%in = OpVariable %ptr_ssbo_int StorageBuffer
+%priv = OpVariable %ptr_private_int Private
+%void_fn = OpTypeFunction %void
+%foo = OpFunction %void None %void_fn
+%entry = OpLabel
+%ld = OpLoad %int %in
+OpStore %priv %ld
+OpReturn
+OpFunctionEnd
+)";
+
+  SetTargetEnv(SPV_ENV_UNIVERSAL_1_4);
+  SinglePassRunAndMatch<PrivateToLocalPass>(text, true);
+}
+
+TEST_F(PrivateToLocalTest, SPV14RemoveFromInterfaceMultipleVariables) {
+  const std::string text = R"(
+; CHECK-NOT: OpEntryPoint GLCompute %foo "foo" %in %priv1 %priv2
+; CHECK: OpEntryPoint GLCompute %foo "foo" %in
+; CHECK: %priv1 = OpVariable {{%\w+}} Function
+; CHECK: %priv2 = OpVariable {{%\w+}} Function
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint GLCompute %foo "foo" %in %priv1 %priv2
+OpExecutionMode %foo LocalSize 1 1 1
+OpName %foo "foo"
+OpName %in "in"
+OpName %priv1 "priv1"
+OpName %priv2 "priv2"
+%void = OpTypeVoid
+%int = OpTypeInt 32 0
+%ptr_ssbo_int = OpTypePointer StorageBuffer %int
+%ptr_private_int = OpTypePointer Private %int
+%in = OpVariable %ptr_ssbo_int StorageBuffer
+%priv1 = OpVariable %ptr_private_int Private
+%priv2 = OpVariable %ptr_private_int Private
+%void_fn = OpTypeFunction %void
+%foo = OpFunction %void None %void_fn
+%entry = OpLabel
+%1 = OpFunctionCall %void %bar1
+%2 = OpFunctionCall %void %bar2
+OpReturn
+OpFunctionEnd
+%bar1 = OpFunction %void None %void_fn
+%3 = OpLabel
+%ld1 = OpLoad %int %in
+OpStore %priv1 %ld1
+OpReturn
+OpFunctionEnd
+%bar2 = OpFunction %void None %void_fn
+%4 = OpLabel
+%ld2 = OpLoad %int %in
+OpStore %priv2 %ld2
+OpReturn
+OpFunctionEnd
+)";
+
+  SetTargetEnv(SPV_ENV_UNIVERSAL_1_4);
+  SinglePassRunAndMatch<PrivateToLocalPass>(text, true);
+}
+
 }  // namespace
 }  // namespace opt
 }  // namespace spvtools

test/val/val_interfaces_test.cpp

@@ -375,6 +375,30 @@ OpFunctionEnd
   EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_UNIVERSAL_1_4));
 }
 
+TEST_F(ValidateInterfacesTest, SPV14MultipleEntryPointsSameFunction) {
+  const std::string text = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint GLCompute %main "main1" %gid
+OpEntryPoint GLCompute %main "main2" %gid
+OpExecutionMode %main LocalSize 1 1 1
+OpDecorate %gid BuiltIn GlobalInvocationId
+%void = OpTypeVoid
+%int = OpTypeInt 32 0
+%int3 = OpTypeVector %int 3
+%ptr_input_int3 = OpTypePointer Input %int3
+%gid = OpVariable %ptr_input_int3 Input
+%void_fn = OpTypeFunction %void
+%main = OpFunction %void None %void_fn
+%entry = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+  CompileSuccessfully(text, SPV_ENV_UNIVERSAL_1_4);
+  EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_UNIVERSAL_1_4));
+}
+
 }  // namespace
 }  // namespace val
 }  // namespace spvtools