Allow getting the base pointer of an image load/store.

In value numbering, we treat loads and stores of images, ie OpImageLoad,
as a memory operation where it is interested in the "base address" of
the instruction.  In those cases, it is an image instruction.

The problem is that `Instruction::GetBaseAddress()` does not account for
the image instructions, so the assert at the end to make sure it found
a valid base address for its addressing mode fails.

The solution is to look at the load/store instruction to determine how
the assertion should be done.

Fixes #1160.

diff --git a/source/opt/instruction.cpp b/source/opt/instruction.cpp
index 182e8b3..45b7d2f 100644 (file)
--- a/source/opt/instruction.cpp
+++ b/source/opt/instruction.cpp
@@ -182,8 +182,21 @@ Instruction* Instruction::GetBaseAddress() const {
     }
   }
 
-  assert(base_inst->IsValidBasePointer() &&
-         "We cannot have a base pointer come from this load");
+  switch (opcode()) {
+    case SpvOpLoad:
+    case SpvOpStore:
+    case SpvOpAccessChain:
+    case SpvOpInBoundsAccessChain:
+    case SpvOpCopyObject:
+      // A load or store through a pointer.
+      assert(base_inst->IsValidBasePointer() &&
+             "We cannot have a base pointer come from this load");
+      break;
+    default:
+      // A load or store of an image.
+      assert(base_inst->IsValidBaseImage() && "We are expecting an image.");
+      break;
+  }
   return base_inst;
 }
 
@@ -425,6 +438,17 @@ bool Instruction::IsValidBasePointer() const {
   return false;
 }
 
+bool Instruction::IsValidBaseImage() const {
+  uint32_t tid = type_id();
+  if (tid == 0) {
+    return false;
+  }
+
+  ir::Instruction* type = context()->get_def_use_mgr()->GetDef(tid);
+  return (type->opcode() == SpvOpTypeImage ||
+          type->opcode() == SpvOpTypeSampledImage);
+}
+
 bool Instruction::IsOpaqueType() const {
   if (opcode() == SpvOpTypeStruct) {
     bool is_opaque = false;

diff --git a/source/opt/instruction.h b/source/opt/instruction.h
index 913695b..49836cd 100644 (file)
--- a/source/opt/instruction.h
+++ b/source/opt/instruction.h
@@ -268,16 +268,17 @@ class Instruction : public utils::IntrusiveNodeBase<Instruction> {
   bool IsReadOnlyLoad() const;
 
   // Returns the instruction that gives the base address of an address
-  // calculation.  The instruction must be a load instruction.  In logical
-  // addressing mode, will return an OpVariable or OpFunctionParameter
-  // instruction. For relaxed logical addressing, it would also return a load of
-  // a pointer to an opaque object.  For physical addressing mode, could return
-  // other types of instructions.
+  // calculation.  The instruction must be a load, as defined by |IsLoad|,
+  // store, copy, or access chain instruction.  In logical addressing mode, will
+  // return an OpVariable or OpFunctionParameter instruction. For relaxed
+  // logical addressing, it would also return a load of a pointer to an opaque
+  // object.  For physical addressing mode, could return other types of
+  // instructions.
   Instruction* GetBaseAddress() const;
 
-  // Returns true if the instruction is a load from memory into a result id. It
-  // considers only core instructions. Memory-to-memory instructions are not
-  // considered loads.
+  // Returns true if the instruction loads from memory or samples an image, and
+  // stores the result into an id. It considers only core instructions.
+  // Memory-to-memory instructions are not considered loads.
   inline bool IsLoad() const;
 
   // Returns true if the instruction declares a variable that is read-only.
@@ -364,6 +365,10 @@ class Instruction : public utils::IntrusiveNodeBase<Instruction> {
   // 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;
+
   IRContext* context_;  // IR Context
   SpvOp opcode_;        // Opcode
   uint32_t type_id_;    // Result type id. A value of 0 means no result type id.

diff --git a/test/opt/instruction_test.cpp b/test/opt/instruction_test.cpp
index a39fa1b..0a632a9 100644 (file)
--- a/test/opt/instruction_test.cpp
+++ b/test/opt/instruction_test.cpp
@@ -32,6 +32,7 @@ using spvtest::MakeInstruction;
 using ::testing::Eq;
 using DescriptorTypeTest = PassTest<::testing::Test>;
 using OpaqueTypeTest = PassTest<::testing::Test>;
+using GetBaseTest = PassTest<::testing::Test>;
 
 TEST(InstructionTest, CreateTrivial) {
   Instruction empty;
@@ -577,4 +578,81 @@ TEST_F(OpaqueTypeTest, OpaqueStructTypes) {
     EXPECT_TRUE(type->IsOpaqueType());
   }
 }
+
+TEST_F(GetBaseTest, SampleImage) {
+  const std::string text = R"(
+               OpCapability Shader
+          %1 = OpExtInstImport "GLSL.std.450"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Fragment %2 "main"
+               OpExecutionMode %2 OriginUpperLeft
+               OpSource GLSL 430
+               OpName %3 "myStorageImage"
+               OpDecorate %3 DescriptorSet 0
+               OpDecorate %3 Binding 0
+          %4 = OpTypeVoid
+          %5 = OpTypeFunction %4
+          %6 = OpTypeFloat 32
+          %7 = OpTypeVector %6 2
+          %8 = OpTypeVector %6 4
+          %9 = OpConstant %6 0
+         %10 = OpConstantComposite %7 %9 %9
+         %11 = OpTypeImage %6 2D 0 0 0 1 R32f
+         %12 = OpTypePointer UniformConstant %11
+          %3 = OpVariable %12 UniformConstant
+         %13 = OpTypeSampledImage %11
+         %14 = OpTypeSampler
+         %15 = OpTypePointer UniformConstant %14
+         %16 = OpVariable %15 UniformConstant
+          %2 = OpFunction %4 None %5
+         %17 = OpLabel
+         %18 = OpLoad %11 %3
+         %19 = OpLoad %14 %16
+         %20 = OpSampledImage %13 %18 %19
+         %21 = OpImageSampleImplicitLod %8 %20 %10
+               OpReturn
+               OpFunctionEnd
+)";
+
+  std::unique_ptr<ir::IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_2, nullptr, text);
+  Instruction* load = context->get_def_use_mgr()->GetDef(21);
+  Instruction* base = context->get_def_use_mgr()->GetDef(20);
+  EXPECT_TRUE(load->GetBaseAddress() == base);
+}
+
+TEST_F(GetBaseTest, ImageRead) {
+  const std::string text = R"(
+               OpCapability Shader
+          %1 = OpExtInstImport "GLSL.std.450"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Fragment %2 "main"
+               OpExecutionMode %2 OriginUpperLeft
+               OpSource GLSL 430
+               OpName %3 "myStorageImage"
+               OpDecorate %3 DescriptorSet 0
+               OpDecorate %3 Binding 0
+          %4 = OpTypeVoid
+          %5 = OpTypeFunction %4
+          %6 = OpTypeInt 32 0
+          %7 = OpTypeVector %6 2
+          %8 = OpConstant %6 0
+          %9 = OpConstantComposite %7 %8 %8
+         %10 = OpTypeImage %6 2D 0 0 0 2 R32f
+         %11 = OpTypePointer UniformConstant %10
+          %3 = OpVariable %11 UniformConstant
+          %2 = OpFunction %4 None %5
+         %12 = OpLabel
+         %13 = OpLoad %10 %3
+         %14 = OpImageRead %6 %13 %9
+               OpReturn
+               OpFunctionEnd
+)";
+
+  std::unique_ptr<ir::IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_2, nullptr, text);
+  Instruction* load = context->get_def_use_mgr()->GetDef(14);
+  Instruction* base = context->get_def_use_mgr()->GetDef(13);
+  EXPECT_TRUE(load->GetBaseAddress() == base);
+}
 }  // anonymous namespace