Add variable pointer support to IsValidBasePointer

Fixes #1729 * Adds supported opcodes to IsValidBasePointer() enable by VariablePointers and VariablePointersStorageBuffer capabilities * Added tests
2024-11-27 09:32:08 +00:00 · 2018-07-19 13:52:24 -04:00 · 2018-07-19 13:52:24 -04:00 · 3c19651733
commit 3c19651733
parent 28199b80b7
3 changed files with 472 additions and 7 deletions
--- a/source/opt/instruction.cpp
+++ b/source/opt/instruction.cpp
@ -419,7 +419,8 @@ bool Instruction::IsValidBasePointer() const {
    return false;
  }

-  if (context()->get_feature_mgr()->HasCapability(SpvCapabilityAddresses)) {
+  auto feature_mgr = context()->get_feature_mgr();
+  if (feature_mgr->HasCapability(SpvCapabilityAddresses)) {
    // TODO: The rules here could be more restrictive.
    return true;
  }
@ -428,6 +429,25 @@ bool Instruction::IsValidBasePointer() const {
    return true;
  }

+  // With variable pointers, there are more valid base pointer objects.
+  // Variable pointers implicitly declares Variable pointers storage buffer.
+  SpvStorageClass storage_class =
+      static_cast<SpvStorageClass>(type->GetSingleWordInOperand(0));
+  if ((feature_mgr->HasCapability(SpvCapabilityVariablePointersStorageBuffer) &&
+       storage_class == SpvStorageClassStorageBuffer) ||
+      (feature_mgr->HasCapability(SpvCapabilityVariablePointers) &&
+       storage_class == SpvStorageClassWorkgroup)) {
+    switch (opcode()) {
+      case SpvOpPhi:
+      case SpvOpSelect:
+      case SpvOpFunctionCall:
+      case SpvOpConstantNull:
+        return true;
+      default:
+        break;
+    }
+  }
+
  uint32_t pointee_type_id = type->GetSingleWordInOperand(1);
  Instruction* pointee_type_inst =
      context()->get_def_use_mgr()->GetDef(pointee_type_id);
--- a/source/opt/instruction.h
+++ b/source/opt/instruction.h
@ -415,6 +415,12 @@ class Instruction : public utils::IntrusiveNodeBase<Instruction> {
  // Return true if the only effect of this instructions is the result.
  bool IsOpcodeSafeToDelete() const;

+  // Returns true if it is valid to use the result of |inst| as the base
+  // pointer for a load or store.  In this case, valid is defined by the relaxed
+  // logical addressing rules when using logical addressing.  Normal validation
+  // rules for physical addressing.
+  bool IsValidBasePointer() const;
+
 private:
  // Returns the total count of result type id and result id.
  uint32_t TypeResultIdCount() const {
@ -427,12 +433,6 @@ class Instruction : public utils::IntrusiveNodeBase<Instruction> {
  bool IsReadOnlyVariableShaders() const;
  bool IsReadOnlyVariableKernel() const;

-  // Returns true if it is valid to use the result of |inst| as the base
-  // pointer for a load or store.  In this case, valid is defined by the relaxed
-  // logical addressing rules when using logical addressing.  Normal validation
-  // rules for physical addressing.
-  bool IsValidBasePointer() const;
-
  // Returns true if the result of |inst| can be used as the base image for an
  // instruction that samples a image, reads an image, or writes to an image.
  bool IsValidBaseImage() const;
--- a/test/opt/instruction_test.cpp
+++ b/test/opt/instruction_test.cpp
@ -31,6 +31,7 @@ using ::testing::Eq;
 using DescriptorTypeTest = PassTest<::testing::Test>;
 using OpaqueTypeTest = PassTest<::testing::Test>;
 using GetBaseTest = PassTest<::testing::Test>;
+using ValidBasePointerTest = PassTest<::testing::Test>;

 TEST(InstructionTest, CreateTrivial) {
  Instruction empty;
@ -654,6 +655,450 @@ TEST_F(GetBaseTest, ImageRead) {
  EXPECT_TRUE(load->GetBaseAddress() == base);
 }

+TEST_F(ValidBasePointerTest, OpSelectBadNoVariablePointersStorageBuffer) {
+  const std::string text = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %1 "func"
+%2 = OpTypeVoid
+%3 = OpTypeInt 32 0
+%4 = OpTypePointer StorageBuffer %3
+%5 = OpVariable %4 StorageBuffer
+%6 = OpTypeFunction %2
+%7 = OpTypeBool
+%8 = OpConstantTrue %7
+%1 = OpFunction %2 None %6
+%9 = OpLabel
+%10 = OpSelect %4 %8 %5 %5
+OpReturn
+OpFunctionEnd
+)";
+
+  std::unique_ptr<IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_3, nullptr, text);
+  EXPECT_NE(context, nullptr);
+  Instruction* select = context->get_def_use_mgr()->GetDef(10);
+  EXPECT_NE(select, nullptr);
+  EXPECT_FALSE(select->IsValidBasePointer());
+}
+
+TEST_F(ValidBasePointerTest, OpSelectBadNoVariablePointers) {
+  const std::string text = R"(
+OpCapability Shader
+OpCapability VariablePointersStorageBuffer
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %1 "func"
+%2 = OpTypeVoid
+%3 = OpTypeInt 32 0
+%4 = OpTypePointer Workgroup %3
+%5 = OpVariable %4 Workgroup
+%6 = OpTypeFunction %2
+%7 = OpTypeBool
+%8 = OpConstantTrue %7
+%1 = OpFunction %2 None %6
+%9 = OpLabel
+%10 = OpSelect %4 %8 %5 %5
+OpReturn
+OpFunctionEnd
+)";
+
+  std::unique_ptr<IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_3, nullptr, text);
+  EXPECT_NE(context, nullptr);
+  Instruction* select = context->get_def_use_mgr()->GetDef(10);
+  EXPECT_NE(select, nullptr);
+  EXPECT_FALSE(select->IsValidBasePointer());
+}
+
+TEST_F(ValidBasePointerTest, OpSelectGoodVariablePointersStorageBuffer) {
+  const std::string text = R"(
+OpCapability Shader
+OpCapability VariablePointersStorageBuffer
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %1 "func"
+%2 = OpTypeVoid
+%3 = OpTypeInt 32 0
+%4 = OpTypePointer StorageBuffer %3
+%5 = OpVariable %4 StorageBuffer
+%6 = OpTypeFunction %2
+%7 = OpTypeBool
+%8 = OpConstantTrue %7
+%1 = OpFunction %2 None %6
+%9 = OpLabel
+%10 = OpSelect %4 %8 %5 %5
+OpReturn
+OpFunctionEnd
+)";
+
+  std::unique_ptr<IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_3, nullptr, text);
+  EXPECT_NE(context, nullptr);
+  Instruction* select = context->get_def_use_mgr()->GetDef(10);
+  EXPECT_NE(select, nullptr);
+  EXPECT_TRUE(select->IsValidBasePointer());
+}
+
+TEST_F(ValidBasePointerTest, OpSelectGoodVariablePointers) {
+  const std::string text = R"(
+OpCapability Shader
+OpCapability VariablePointers
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %1 "func"
+%2 = OpTypeVoid
+%3 = OpTypeInt 32 0
+%4 = OpTypePointer Workgroup %3
+%5 = OpVariable %4 Workgroup
+%6 = OpTypeFunction %2
+%7 = OpTypeBool
+%8 = OpConstantTrue %7
+%1 = OpFunction %2 None %6
+%9 = OpLabel
+%10 = OpSelect %4 %8 %5 %5
+OpReturn
+OpFunctionEnd
+)";
+
+  std::unique_ptr<IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_3, nullptr, text);
+  EXPECT_NE(context, nullptr);
+  Instruction* select = context->get_def_use_mgr()->GetDef(10);
+  EXPECT_NE(select, nullptr);
+  EXPECT_TRUE(select->IsValidBasePointer());
+}
+
+TEST_F(ValidBasePointerTest, OpConstantNullBadNoVariablePointersStorageBuffer) {
+  const std::string text = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %1 "func"
+%2 = OpTypeVoid
+%3 = OpTypeInt 32 0
+%4 = OpTypePointer StorageBuffer %3
+%5 = OpConstantNull %4
+%6 = OpTypeFunction %2
+%1 = OpFunction %2 None %6
+%7 = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+  std::unique_ptr<IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_3, nullptr, text);
+  EXPECT_NE(context, nullptr);
+  Instruction* null_inst = context->get_def_use_mgr()->GetDef(5);
+  EXPECT_NE(null_inst, nullptr);
+  EXPECT_FALSE(null_inst->IsValidBasePointer());
+}
+
+TEST_F(ValidBasePointerTest, OpConstantNullBadNoVariablePointers) {
+  const std::string text = R"(
+OpCapability Shader
+OpCapability VariablePointersStorageBuffer
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %1 "func"
+%2 = OpTypeVoid
+%3 = OpTypeInt 32 0
+%4 = OpTypePointer Workgroup %3
+%5 = OpConstantNull %4
+%6 = OpTypeFunction %2
+%1 = OpFunction %2 None %6
+%7 = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+  std::unique_ptr<IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_3, nullptr, text);
+  EXPECT_NE(context, nullptr);
+  Instruction* null_inst = context->get_def_use_mgr()->GetDef(5);
+  EXPECT_NE(null_inst, nullptr);
+  EXPECT_FALSE(null_inst->IsValidBasePointer());
+}
+
+TEST_F(ValidBasePointerTest, OpConstantNullGoodVariablePointersStorageBuffer) {
+  const std::string text = R"(
+OpCapability Shader
+OpCapability VariablePointersStorageBuffer
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %1 "func"
+%2 = OpTypeVoid
+%3 = OpTypeInt 32 0
+%4 = OpTypePointer StorageBuffer %3
+%5 = OpConstantNull %4
+%6 = OpTypeFunction %2
+%1 = OpFunction %2 None %6
+%9 = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+  std::unique_ptr<IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_3, nullptr, text);
+  EXPECT_NE(context, nullptr);
+  Instruction* null_inst = context->get_def_use_mgr()->GetDef(5);
+  EXPECT_NE(null_inst, nullptr);
+  EXPECT_TRUE(null_inst->IsValidBasePointer());
+}
+
+TEST_F(ValidBasePointerTest, OpConstantNullGoodVariablePointers) {
+  const std::string text = R"(
+OpCapability Shader
+OpCapability VariablePointers
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %1 "func"
+%2 = OpTypeVoid
+%3 = OpTypeInt 32 0
+%4 = OpTypePointer Workgroup %3
+%5 = OpConstantNull %4
+%6 = OpTypeFunction %2
+%1 = OpFunction %2 None %6
+%7 = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+  std::unique_ptr<IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_3, nullptr, text);
+  EXPECT_NE(context, nullptr);
+  Instruction* null_inst = context->get_def_use_mgr()->GetDef(5);
+  EXPECT_NE(null_inst, nullptr);
+  EXPECT_TRUE(null_inst->IsValidBasePointer());
+}
+
+TEST_F(ValidBasePointerTest, OpPhiBadNoVariablePointersStorageBuffer) {
+  const std::string text = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %1 "func"
+%2 = OpTypeVoid
+%3 = OpTypeInt 32 0
+%4 = OpTypePointer StorageBuffer %3
+%5 = OpVariable %4 StorageBuffer
+%6 = OpTypeFunction %2
+%1 = OpFunction %2 None %6
+%7 = OpLabel
+OpBranch %8
+%8 = OpLabel
+%9 = OpPhi %4 %5 %7
+OpReturn
+OpFunctionEnd
+)";
+
+  std::unique_ptr<IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_3, nullptr, text);
+  EXPECT_NE(context, nullptr);
+  Instruction* phi = context->get_def_use_mgr()->GetDef(9);
+  EXPECT_NE(phi, nullptr);
+  EXPECT_FALSE(phi->IsValidBasePointer());
+}
+
+TEST_F(ValidBasePointerTest, OpPhiBadNoVariablePointers) {
+  const std::string text = R"(
+OpCapability Shader
+OpCapability VariablePointersStorageBuffer
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %1 "func"
+%2 = OpTypeVoid
+%3 = OpTypeInt 32 0
+%4 = OpTypePointer Workgroup %3
+%5 = OpVariable %4 Workgroup
+%6 = OpTypeFunction %2
+%1 = OpFunction %2 None %6
+%7 = OpLabel
+OpBranch %8
+%8 = OpLabel
+%9 = OpPhi %4 %5 %7
+OpReturn
+OpFunctionEnd
+)";
+
+  std::unique_ptr<IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_3, nullptr, text);
+  EXPECT_NE(context, nullptr);
+  Instruction* phi = context->get_def_use_mgr()->GetDef(9);
+  EXPECT_NE(phi, nullptr);
+  EXPECT_FALSE(phi->IsValidBasePointer());
+}
+
+TEST_F(ValidBasePointerTest, OpPhiGoodVariablePointersStorageBuffer) {
+  const std::string text = R"(
+OpCapability Shader
+OpCapability VariablePointersStorageBuffer
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %1 "func"
+%2 = OpTypeVoid
+%3 = OpTypeInt 32 0
+%4 = OpTypePointer StorageBuffer %3
+%5 = OpVariable %4 StorageBuffer
+%6 = OpTypeFunction %2
+%1 = OpFunction %2 None %6
+%7 = OpLabel
+OpBranch %8
+%8 = OpLabel
+%9 = OpPhi %4 %5 %7
+OpReturn
+OpFunctionEnd
+)";
+
+  std::unique_ptr<IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_3, nullptr, text);
+  EXPECT_NE(context, nullptr);
+  Instruction* phi = context->get_def_use_mgr()->GetDef(9);
+  EXPECT_NE(phi, nullptr);
+  EXPECT_TRUE(phi->IsValidBasePointer());
+}
+
+TEST_F(ValidBasePointerTest, OpPhiGoodVariablePointers) {
+  const std::string text = R"(
+OpCapability Shader
+OpCapability VariablePointers
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %1 "func"
+%2 = OpTypeVoid
+%3 = OpTypeInt 32 0
+%4 = OpTypePointer Workgroup %3
+%5 = OpVariable %4 Workgroup
+%6 = OpTypeFunction %2
+%1 = OpFunction %2 None %6
+%7 = OpLabel
+OpBranch %8
+%8 = OpLabel
+%9 = OpPhi %4 %5 %7
+OpReturn
+OpFunctionEnd
+)";
+
+  std::unique_ptr<IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_3, nullptr, text);
+  EXPECT_NE(context, nullptr);
+  Instruction* phi = context->get_def_use_mgr()->GetDef(9);
+  EXPECT_NE(phi, nullptr);
+  EXPECT_TRUE(phi->IsValidBasePointer());
+}
+
+TEST_F(ValidBasePointerTest, OpFunctionCallBadNoVariablePointersStorageBuffer) {
+  const std::string text = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %1 "func"
+%2 = OpTypeVoid
+%3 = OpTypeInt 32 0
+%4 = OpTypePointer StorageBuffer %3
+%5 = OpConstantNull %4
+%6 = OpTypeFunction %2
+%7 = OpTypeFunction %4
+%1 = OpFunction %2 None %6
+%8 = OpLabel
+%9 = OpFunctionCall %4 %10
+OpReturn
+OpFunctionEnd
+%10 = OpFunction %4 None %7
+%11 = OpLabel
+OpReturnValue %5
+OpFunctionEnd
+)";
+
+  std::unique_ptr<IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_3, nullptr, text);
+  EXPECT_NE(context, nullptr);
+  Instruction* null_inst = context->get_def_use_mgr()->GetDef(9);
+  EXPECT_NE(null_inst, nullptr);
+  EXPECT_FALSE(null_inst->IsValidBasePointer());
+}
+
+TEST_F(ValidBasePointerTest, OpFunctionCallBadNoVariablePointers) {
+  const std::string text = R"(
+OpCapability Shader
+OpCapability VariablePointersStorageBuffer
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %1 "func"
+%2 = OpTypeVoid
+%3 = OpTypeInt 32 0
+%4 = OpTypePointer Workgroup %3
+%5 = OpConstantNull %4
+%6 = OpTypeFunction %2
+%7 = OpTypeFunction %4
+%1 = OpFunction %2 None %6
+%8 = OpLabel
+%9 = OpFunctionCall %4 %10
+OpReturn
+OpFunctionEnd
+%10 = OpFunction %4 None %7
+%11 = OpLabel
+OpReturnValue %5
+OpFunctionEnd
+)";
+
+  std::unique_ptr<IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_3, nullptr, text);
+  EXPECT_NE(context, nullptr);
+  Instruction* null_inst = context->get_def_use_mgr()->GetDef(9);
+  EXPECT_NE(null_inst, nullptr);
+  EXPECT_FALSE(null_inst->IsValidBasePointer());
+}
+
+TEST_F(ValidBasePointerTest, OpFunctionCallGoodVariablePointersStorageBuffer) {
+  const std::string text = R"(
+OpCapability Shader
+OpCapability VariablePointersStorageBuffer
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %1 "func"
+%2 = OpTypeVoid
+%3 = OpTypeInt 32 0
+%4 = OpTypePointer StorageBuffer %3
+%5 = OpConstantNull %4
+%6 = OpTypeFunction %2
+%7 = OpTypeFunction %4
+%1 = OpFunction %2 None %6
+%8 = OpLabel
+%9 = OpFunctionCall %4 %10
+OpReturn
+OpFunctionEnd
+%10 = OpFunction %4 None %7
+%11 = OpLabel
+OpReturnValue %5
+OpFunctionEnd
+)";
+
+  std::unique_ptr<IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_3, nullptr, text);
+  EXPECT_NE(context, nullptr);
+  Instruction* null_inst = context->get_def_use_mgr()->GetDef(9);
+  EXPECT_NE(null_inst, nullptr);
+  EXPECT_TRUE(null_inst->IsValidBasePointer());
+}
+
+TEST_F(ValidBasePointerTest, OpFunctionCallGoodVariablePointers) {
+  const std::string text = R"(
+OpCapability Shader
+OpCapability VariablePointers
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %1 "func"
+%2 = OpTypeVoid
+%3 = OpTypeInt 32 0
+%4 = OpTypePointer Workgroup %3
+%5 = OpConstantNull %4
+%6 = OpTypeFunction %2
+%7 = OpTypeFunction %4
+%1 = OpFunction %2 None %6
+%8 = OpLabel
+%9 = OpFunctionCall %4 %10
+OpReturn
+OpFunctionEnd
+%10 = OpFunction %4 None %7
+%11 = OpLabel
+OpReturnValue %5
+OpFunctionEnd
+)";
+
+  std::unique_ptr<IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_3, nullptr, text);
+  EXPECT_NE(context, nullptr);
+  Instruction* null_inst = context->get_def_use_mgr()->GetDef(9);
+  EXPECT_NE(null_inst, nullptr);
+  EXPECT_TRUE(null_inst->IsValidBasePointer());
+}
+
 }  // namespace
 }  // namespace opt
 }  // namespace spvtools