Validator support for Variable Pointer extension.

If the variable_pointer extension is used:

* OpLoad's pointer argument may be the result of any of the following:
 * OpSelect
 * OpPhi
 * OpFunctionCall
 * OpPtrAccessChain
 * OpCopyObject
 * OpLoad
 * OpConstantNull

* Return value of a function may be a pointer.

* It is valid to use a pointer as the return value of a function.

* OpStore should allow a variable pointer argument.

diff --git a/source/opcode.cpp b/source/opcode.cpp
index 9f51b95..e3922f4 100644 (file)
--- a/source/opcode.cpp
+++ b/source/opcode.cpp
@@ -230,6 +230,26 @@ int32_t spvOpcodeIsComposite(const SpvOp opcode) {
   }
 }
 
+bool spvOpcodeReturnsLogicalVariablePointer(const SpvOp opcode) {
+  switch (opcode) {
+    case SpvOpVariable:
+    case SpvOpAccessChain:
+    case SpvOpInBoundsAccessChain:
+    case SpvOpFunctionParameter:
+    case SpvOpImageTexelPointer:
+    case SpvOpCopyObject:
+    case SpvOpSelect:
+    case SpvOpPhi:
+    case SpvOpFunctionCall:
+    case SpvOpPtrAccessChain:
+    case SpvOpLoad:
+    case SpvOpConstantNull:
+      return true;
+    default:
+      return false;
+  }
+}
+
 int32_t spvOpcodeReturnsLogicalPointer(const SpvOp opcode) {
   switch (opcode) {
     case SpvOpVariable:

diff --git a/source/opcode.h b/source/opcode.h
index 9742cbc..a54a96d 100644 (file)
--- a/source/opcode.h
+++ b/source/opcode.h
@@ -79,6 +79,10 @@ int32_t spvOpcodeIsComposite(const SpvOp opcode);
 // addressing model. Returns zero if false, non-zero otherwise.
 int32_t spvOpcodeReturnsLogicalPointer(const SpvOp opcode);
 
+// Returns whether the given opcode could result in a pointer or a variable
+// pointer when using the logical addressing model.
+bool spvOpcodeReturnsLogicalVariablePointer(const SpvOp opcode);
+
 // Determines if the given opcode generates a type. Returns zero if false,
 // non-zero otherwise.
 int32_t spvOpcodeGeneratesType(SpvOp opcode);

diff --git a/source/val/validation_state.cpp b/source/val/validation_state.cpp
index 12e864d..86e2d98 100644 (file)
--- a/source/val/validation_state.cpp
+++ b/source/val/validation_state.cpp
@@ -300,6 +300,14 @@ void ValidationState_t::RegisterCapability(SpvCapability cap) {
       features_.declare_int16_type = true;
       features_.declare_float16_type = true;
       features_.free_fp_rounding_mode = true;
+      break;
+    case SpvCapabilityVariablePointers:
+      features_.variable_pointers = true;
+      features_.variable_pointers_uniform_buffer_block = true;
+      break;
+    case SpvCapabilityVariablePointersUniformBufferBlock:
+      features_.variable_pointers_uniform_buffer_block = true;
+      break;
     default:
       break;
   }

diff --git a/source/val/validation_state.h b/source/val/validation_state.h
index a61fbab..d130c0f 100644 (file)
--- a/source/val/validation_state.h
+++ b/source/val/validation_state.h
@@ -62,6 +62,12 @@ class ValidationState_t {
     bool free_fp_rounding_mode = false;  // Allow the FPRoundingMode decoration
                                          // and its vaules to be used without
                                          // requiring any capability
+
+    // Allow functionalities enabled by VariablePointers capability.
+    bool variable_pointers = false;
+    // Allow functionalities enabled by VariablePointersUniformBufferBlock
+    // capability.
+    bool variable_pointers_uniform_buffer_block = false;
   };
 
   ValidationState_t(const spv_const_context context,

diff --git a/source/validate_id.cpp b/source/validate_id.cpp
index 039c9eb..f150e56 100644 (file)
--- a/source/validate_id.cpp
+++ b/source/validate_id.cpp
@@ -1116,10 +1116,17 @@ bool idUsage::isValid<SpvOpLoad>(const spv_instruction_t* inst,
                           << inst->words[resultTypeIndex] << "' is not defind.";
     return false;
   }
+  const bool uses_variable_pointer =
+      module_.features().variable_pointers ||
+      module_.features().variable_pointers_uniform_buffer_block;
   auto pointerIndex = 3;
   auto pointer = module_.FindDef(inst->words[pointerIndex]);
-  if (!pointer || (addressingModel == SpvAddressingModelLogical &&
-                   !spvOpcodeReturnsLogicalPointer(pointer->opcode()))) {
+  if (!pointer ||
+      (addressingModel == SpvAddressingModelLogical &&
+       ((!uses_variable_pointer &&
+         !spvOpcodeReturnsLogicalPointer(pointer->opcode())) ||
+        (uses_variable_pointer &&
+         !spvOpcodeReturnsLogicalVariablePointer(pointer->opcode()))))) {
     DIAG(pointerIndex) << "OpLoad Pointer <id> '" << inst->words[pointerIndex]
                        << "' is not a logical pointer.";
     return false;
@@ -1145,10 +1152,17 @@ bool idUsage::isValid<SpvOpLoad>(const spv_instruction_t* inst,
 template <>
 bool idUsage::isValid<SpvOpStore>(const spv_instruction_t* inst,
                                   const spv_opcode_desc) {
-  auto pointerIndex = 1;
+  const bool uses_variable_pointer =
+      module_.features().variable_pointers ||
+      module_.features().variable_pointers_uniform_buffer_block;
+  const auto pointerIndex = 1;
   auto pointer = module_.FindDef(inst->words[pointerIndex]);
-  if (!pointer || (addressingModel == SpvAddressingModelLogical &&
-                   !spvOpcodeReturnsLogicalPointer(pointer->opcode()))) {
+  if (!pointer ||
+      (addressingModel == SpvAddressingModelLogical &&
+       ((!uses_variable_pointer &&
+         !spvOpcodeReturnsLogicalPointer(pointer->opcode())) ||
+        (uses_variable_pointer &&
+         !spvOpcodeReturnsLogicalVariablePointer(pointer->opcode()))))) {
     DIAG(pointerIndex) << "OpStore Pointer <id> '" << inst->words[pointerIndex]
                        << "' is not a logical pointer.";
     return false;
@@ -2404,8 +2418,11 @@ bool idUsage::isValid<SpvOpReturnValue>(const spv_instruction_t* inst,
                      << "' is missing or void.";
     return false;
   }
+  const bool uses_variable_pointer =
+      module_.features().variable_pointers ||
+      module_.features().variable_pointers_uniform_buffer_block;
   if (addressingModel == SpvAddressingModelLogical &&
-      SpvOpTypePointer == valueType->opcode()) {
+      SpvOpTypePointer == valueType->opcode() && !uses_variable_pointer) {
     DIAG(valueIndex)
         << "OpReturnValue value's type <id> '" << value->type_id()
         << "' is a pointer, which is invalid in the Logical addressing model.";

diff --git a/source/validate_instruction.cpp b/source/validate_instruction.cpp
index 49d2113..aafb7b0 100644 (file)
--- a/source/validate_instruction.cpp
+++ b/source/validate_instruction.cpp
@@ -395,9 +395,10 @@ spv_result_t InstructionPass(ValidationState_t& _,
   const SpvOp opcode = static_cast<SpvOp>(inst->opcode);
   if (opcode == SpvOpExtension)
     CheckIfKnownExtension(_, inst);
-  if (opcode == SpvOpCapability)
+  if (opcode == SpvOpCapability) {
     _.RegisterCapability(
         static_cast<SpvCapability>(inst->words[inst->operands[0].offset]));
+  }
   if (opcode == SpvOpMemoryModel) {
     _.set_addressing_model(
         static_cast<SpvAddressingModel>(inst->words[inst->operands[0].offset]));

diff --git a/test/val/val_id_test.cpp b/test/val/val_id_test.cpp
index 754420a..ba8cf9f 100644 (file)
--- a/test/val/val_id_test.cpp
+++ b/test/val/val_id_test.cpp
@@ -1805,6 +1805,163 @@ TEST_F(ValidateIdWithMessage, OpLoadGood) {
   CompileSuccessfully(spirv.c_str());
   EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
 }
+
+// TODO: Add tests that exercise VariablePointersUniformBufferBlock instead of
+// VariablePointers.
+void createVariablePointerSpirvProgram(std::ostringstream* spirv,
+                                       std::string result_strategy,
+                                       bool use_varptr_cap,
+                                       bool add_helper_function) {
+  *spirv << "OpCapability Shader ";
+  if (use_varptr_cap) {
+    *spirv << "OpCapability VariablePointers ";
+    *spirv << "OpExtension \"SPV_KHR_variable_pointers\" ";
+  }
+  *spirv << R"(
+    OpMemoryModel Logical GLSL450
+    OpEntryPoint GLCompute %main "main"
+    %void      = OpTypeVoid
+    %voidf     = OpTypeFunction %void
+    %bool      = OpTypeBool
+    %i32       = OpTypeInt 32 1
+    %f32       = OpTypeFloat 32
+    %f32ptr    = OpTypePointer Uniform %f32
+    %i         = OpConstant %i32 1
+    %zero      = OpConstant %i32 0
+    %float_1   = OpConstant %f32 1.0
+    %ptr1      = OpVariable %f32ptr Uniform
+    %ptr2      = OpVariable %f32ptr Uniform
+  )";
+  if (add_helper_function) {
+    *spirv << R"(
+      ; ////////////////////////////////////////////////////////////
+      ;;;; Function that returns a pointer
+      ; ////////////////////////////////////////////////////////////
+      %selector_func_type  = OpTypeFunction %f32ptr %bool %f32ptr %f32ptr
+      %choose_input_func   = OpFunction %f32ptr None %selector_func_type
+      %is_neg_param        = OpFunctionParameter %bool
+      %first_ptr_param     = OpFunctionParameter %f32ptr
+      %second_ptr_param    = OpFunctionParameter %f32ptr
+      %selector_func_begin = OpLabel
+      %result_ptr          = OpSelect %f32ptr %is_neg_param %first_ptr_param %second_ptr_param
+      OpReturnValue %result_ptr
+      OpFunctionEnd
+    )";
+  }
+  *spirv << R"(
+    %main      = OpFunction %void None %voidf
+    %label     = OpLabel
+  )";
+  *spirv << result_strategy;
+  *spirv << R"(
+    OpReturn
+    OpFunctionEnd
+  )";
+}
+
+// With the VariablePointer Capability, OpLoad should allow loading a
+// VaiablePointer. In this test the variable pointer is obtained by an OpSelect
+TEST_F(ValidateIdWithMessage, OpLoadVarPtrOpSelectGood) {
+  std::string result_strategy = R"(
+    %isneg     = OpSLessThan %bool %i %zero
+    %varptr    = OpSelect %f32ptr %isneg %ptr1 %ptr2
+    %result    = OpLoad %f32 %varptr
+  )";
+
+  std::ostringstream spirv;
+  createVariablePointerSpirvProgram(&spirv, result_strategy,
+                                    true /* Add VariablePointers Capability? */,
+                                    false /* Use Helper Function? */);
+  CompileSuccessfully(spirv.str());
+  EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
+}
+
+// Without the VariablePointers Capability, OpLoad will not allow loading
+// through a variable pointer.
+TEST_F(ValidateIdWithMessage, OpLoadVarPtrOpSelectBad) {
+  std::string result_strategy = R"(
+    %isneg     = OpSLessThan %bool %i %zero
+    %varptr    = OpSelect %f32ptr %isneg %ptr1 %ptr2
+    %result    = OpLoad %f32 %varptr
+  )";
+
+  std::ostringstream spirv;
+  createVariablePointerSpirvProgram(&spirv, result_strategy,
+                                    false /* Add VariablePointers Capability?*/,
+                                    false /* Use Helper Function? */);
+  CompileSuccessfully(spirv.str());
+  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("is not a logical pointer."));
+}
+
+// With the VariablePointer Capability, OpLoad should allow loading a
+// VaiablePointer. In this test the variable pointer is obtained by an OpPhi
+TEST_F(ValidateIdWithMessage, OpLoadVarPtrOpPhiGood) {
+  std::string result_strategy = R"(
+    %is_neg      = OpSLessThan %bool %i %zero
+    OpSelectionMerge %end_label None
+    OpBranchConditional %is_neg %take_ptr_1 %take_ptr_2
+    %take_ptr_1 = OpLabel
+    OpBranch      %end_label
+    %take_ptr_2 = OpLabel
+    OpBranch      %end_label
+    %end_label  = OpLabel
+    %varptr     = OpPhi %f32ptr %ptr1 %take_ptr_1 %ptr2 %take_ptr_2
+    %result     = OpLoad %f32 %varptr
+  )";
+
+  std::ostringstream spirv;
+  createVariablePointerSpirvProgram(&spirv, result_strategy,
+                                    true /* Add VariablePointers Capability?*/,
+                                    false /* Use Helper Function? */);
+  CompileSuccessfully(spirv.str());
+  EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
+}
+
+// Without the VariablePointers Capability, OpLoad will not allow loading
+// through a variable pointer.
+TEST_F(ValidateIdWithMessage, OpLoadVarPtrOpPhiBad) {
+  std::string result_strategy = R"(
+    %is_neg      = OpSLessThan %bool %i %zero
+    OpSelectionMerge %end_label None
+    OpBranchConditional %is_neg %take_ptr_1 %take_ptr_2
+    %take_ptr_1 = OpLabel
+    OpBranch      %end_label
+    %take_ptr_2 = OpLabel
+    OpBranch      %end_label
+    %end_label  = OpLabel
+    %varptr     = OpPhi %f32ptr %ptr1 %take_ptr_1 %ptr2 %take_ptr_2
+    %result     = OpLoad %f32 %varptr
+  )";
+
+  std::ostringstream spirv;
+  createVariablePointerSpirvProgram(&spirv, result_strategy,
+                                    false /* Add VariablePointers Capability?*/,
+                                    false /* Use Helper Function? */);
+  CompileSuccessfully(spirv.str());
+  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("is not a logical pointer"));
+}
+
+// With the VariablePointer Capability, OpLoad should allow loading through a
+// VaiablePointer. In this test the variable pointer is obtained from an
+// OpFunctionCall (return value from a function)
+TEST_F(ValidateIdWithMessage, OpLoadVarPtrOpFunctionCallGood) {
+  std::ostringstream spirv;
+  std::string result_strategy = R"(
+    %isneg     = OpSLessThan %bool %i %zero
+    %varptr    = OpFunctionCall %f32ptr %choose_input_func %isneg %ptr1 %ptr2
+    %result    = OpLoad %f32 %varptr
+  )";
+
+  createVariablePointerSpirvProgram(&spirv,
+                                    result_strategy,
+                                    true /* Add VariablePointers Capability?*/,
+                                    true /* Use Helper Function? */);
+  CompileSuccessfully(spirv.str());
+  EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
+}
+
 TEST_F(ValidateIdWithMessage, OpLoadResultTypeBad) {
   string spirv = kGLSL450MemoryModel + R"(
 %1 = OpTypeVoid
@@ -1927,6 +2084,41 @@ TEST_F(ValidateIdWithMessage, OpStoreLogicalPointerBad) {
               HasSubstr("OpStore Pointer <id> '10' is not a logical pointer."));
 }
 
+// Without the VariablePointer Capability, OpStore should may not store
+// through a variable pointer.
+TEST_F(ValidateIdWithMessage, OpStoreVarPtrBad) {
+  std::string result_strategy = R"(
+    %isneg     = OpSLessThan %bool %i %zero
+    %varptr    = OpSelect %f32ptr %isneg %ptr1 %ptr2
+                 OpStore %varptr %float_1
+  )";
+
+  std::ostringstream spirv;
+  createVariablePointerSpirvProgram(
+      &spirv, result_strategy, false /* Add VariablePointers Capability? */,
+      false /* Use Helper Function? */);
+  CompileSuccessfully(spirv.str());
+  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("is not a logical pointer."));
+}
+
+// With the VariablePointer Capability, OpStore should allow storing through a
+// variable pointer.
+TEST_F(ValidateIdWithMessage, OpStoreVarPtrGood) {
+  std::string result_strategy = R"(
+    %isneg     = OpSLessThan %bool %i %zero
+    %varptr    = OpSelect %f32ptr %isneg %ptr1 %ptr2
+                 OpStore %varptr %float_1
+  )";
+
+  std::ostringstream spirv;
+  createVariablePointerSpirvProgram(&spirv, result_strategy,
+                                    true /* Add VariablePointers Capability? */,
+                                    false /* Use Helper Function? */);
+  CompileSuccessfully(spirv.str());
+  EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
+}
+
 TEST_F(ValidateIdWithMessage, OpStoreObjectGood) {
   string spirv = kGLSL450MemoryModel + R"(
 %1 = OpTypeVoid
@@ -3614,6 +3806,33 @@ TEST_F(ValidateIdWithMessage, OpReturnValueIsVariableInLogical) {
                         "which is invalid in the Logical addressing model."));
 }
 
+// With the VariablePointer Capability, the return value of a function is
+// allowed to be a pointer.
+TEST_F(ValidateIdWithMessage, OpReturnValueVarPtrGood) {
+  std::ostringstream spirv;
+  createVariablePointerSpirvProgram(&spirv,
+                                    ""   /* Instructions to add to "main" */,
+                                    true /* Add VariablePointers Capability?*/,
+                                    true /* Use Helper Function? */);
+  CompileSuccessfully(spirv.str());
+  EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
+}
+
+// Without the VariablePointer Capability, the return value of a function is
+// *not* allowed to be a pointer.
+TEST_F(ValidateIdWithMessage, OpReturnValueVarPtrBad) {
+  std::ostringstream spirv;
+  createVariablePointerSpirvProgram(&spirv,
+                                    ""    /* Instructions to add to "main" */,
+                                    false /* Add VariablePointers Capability?*/,
+                                    true  /* Use Helper Function? */);
+  CompileSuccessfully(spirv.str());
+  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("OpReturnValue value's type <id> '7' is a pointer, "
+                        "which is invalid in the Logical addressing model."));
+}
+
 // TODO: enable when this bug is fixed:
 // https://cvs.khronos.org/bugzilla/show_bug.cgi?id=15404
 TEST_F(ValidateIdWithMessage, DISABLED_OpReturnValueIsFunction) {