Compute value number when the value table is constructed.
authorSteven Perron <stevenperron@google.com>
Mon, 27 Nov 2017 20:07:34 +0000 (15:07 -0500)
committerSteven Perron <stevenperron@google.com>
Mon, 4 Dec 2017 16:02:04 +0000 (11:02 -0500)
Computing the value numbers on demand, as we do now, can lead to
different results depending on the order in which the users asks for
the value numbers.  To make things more stable, we compute them ahead
of time.

source/opt/local_redundancy_elimination.cpp
source/opt/value_number_table.cpp
source/opt/value_number_table.h
test/opt/value_table_test.cpp

index 093c861..45ba012 100644 (file)
@@ -23,14 +23,10 @@ Pass::Status LocalRedundancyEliminationPass::Process(ir::IRContext* c) {
   InitializeProcessing(c);
 
   bool modified = false;
+  ValueNumberTable vnTable(context());
 
   for (auto& func : *get_module()) {
     for (auto& bb : func) {
-      // Resetting the value number table for every basic block because we just
-      // want the opportunities within a basic block. This will help keep
-      // register pressure down.
-      ValueNumberTable vnTable(context());
-
       // Keeps track of all ids that contain a given value number. We keep
       // track of multiple values because they could have the same value, but
       // different decorations.
@@ -39,7 +35,6 @@ Pass::Status LocalRedundancyEliminationPass::Process(ir::IRContext* c) {
         modified = true;
     }
   }
-
   return (modified ? Status::SuccessWithChange : Status::SuccessWithoutChange);
 }
 
@@ -54,6 +49,11 @@ bool LocalRedundancyEliminationPass::EliminateRedundanciesInBB(
     }
 
     uint32_t value = vnTable->GetValueNumber(inst);
+
+    if (value == 0) {
+      return;
+    }
+
     if (value >= value_to_ids->size()) {
       value_to_ids->resize(value + 1);
     }
index d4dabf2..d0d2876 100644 (file)
 
 #include <algorithm>
 
+#include "cfg.h"
+
 namespace spvtools {
 namespace opt {
 
 uint32_t ValueNumberTable::GetValueNumber(spvtools::ir::Instruction* inst) {
-  // TODO: Need to implement the substitution of operands by their value number
-  // before hashing.
-  // TODO: Implement a normal form for opcodes that commute like integer
-  // addition.  This will let us know that a+b is the same value as b+a.
   assert(inst->result_id() != 0 &&
          "inst must have a result id to get a value number.");
 
   // Check if this instruction already has a value.
-  auto id_to_val = id_to_value_.find(inst->result_id());
-  if (id_to_val != id_to_value_.end()) {
-    return id_to_val->second;
+  auto result_id_to_val = id_to_value_.find(inst->result_id());
+  if (result_id_to_val != id_to_value_.end()) {
+    return result_id_to_val->second;
+  }
+  return 0;
+}
+
+uint32_t ValueNumberTable::AssignValueNumber(ir::Instruction* inst) {
+  // If it already has a value return that.
+  uint32_t value = GetValueNumber(inst);
+  if (value != 0) {
+    return value;
   }
 
   // If the instruction has other side effects, then it must
   // have its own value number.
+  // OpSampledImage and OpImage must remain in the same basic block in which
+  // they are used, because of this we will assign each one it own value number.
   if (!context()->IsCombinatorInstruction(inst)) {
-    uint32_t value_number = TakeNextValueNumber();
-    id_to_value_[inst->result_id()] = value_number;
-    return value_number;
+    value = TakeNextValueNumber();
+    id_to_value_[inst->result_id()] = value;
+    return value;
+  }
+
+  switch (inst->opcode()) {
+    case SpvOpSampledImage:
+    case SpvOpImage:
+    case SpvOpVariable:
+      value = TakeNextValueNumber();
+      id_to_value_[inst->result_id()] = value;
+      return value;
+    default:
+      break;
   }
 
   // If it is a load from memory that can be modified, we have to assume the
@@ -48,24 +68,111 @@ uint32_t ValueNumberTable::GetValueNumber(spvtools::ir::Instruction* inst) {
   // read only.  However, if this is ever relaxed because we analyze stores, we
   // will have to add a new case for volatile loads.
   if (inst->IsLoad() && !inst->IsReadOnlyLoad()) {
-    uint32_t value_number = TakeNextValueNumber();
-    id_to_value_[inst->result_id()] = value_number;
-    return value_number;
+    value = TakeNextValueNumber();
+    id_to_value_[inst->result_id()] = value;
+    return value;
+  }
+
+  // When we copy an object, the value numbers should be the same.
+  if (inst->opcode() == SpvOpCopyObject) {
+    value = GetValueNumber(inst->GetSingleWordInOperand(0));
+    if (value != 0) {
+      id_to_value_[inst->result_id()] = value;
+      return value;
+    }
+  }
+
+  // Phi nodes are a type of copy.  If all of the inputs have the same value
+  // number, then we can assign the result of the phi the same value number.
+  if (inst->opcode() == SpvOpPhi) {
+    value = GetValueNumber(inst->GetSingleWordInOperand(0));
+    if (value != 0) {
+      for (uint32_t op = 2; op < inst->NumInOperands(); op += 2) {
+        if (value != GetValueNumber(inst->GetSingleWordInOperand(op))) {
+          value = 0;
+          break;
+        }
+      }
+      if (value != 0) {
+        id_to_value_[inst->result_id()] = value;
+        return value;
+      }
+    }
   }
 
+  // Replace all of the operands by their value number.  The sign bit will be
+  // set to distinguish between an id and a value number.
+  ir::Instruction value_ins(context(), inst->opcode(), inst->type_id(),
+                            inst->result_id(), {});
+  for (uint32_t o = 0; o < inst->NumInOperands(); ++o) {
+    const ir::Operand& op = inst->GetInOperand(o);
+    if (spvIsIdType(op.type)) {
+      uint32_t id_value = op.words[0];
+      auto use_id_to_val = id_to_value_.find(id_value);
+      if (use_id_to_val != id_to_value_.end()) {
+        id_value = (1 << 31) | use_id_to_val->second;
+      }
+      value_ins.AddOperand(ir::Operand(op.type, {id_value}));
+    } else {
+      value_ins.AddOperand(ir::Operand(op.type, op.words));
+    }
+  }
+
+  // TODO: Implement a normal form for opcodes that commute like integer
+  // addition.  This will let us know that a+b is the same value as b+a.
+
   // Otherwise, we check if this value has been computed before.
-  auto value = instruction_to_value_.find(*inst);
-  if (value != instruction_to_value_.end()) {
-    uint32_t value_number = id_to_value_[value->first.result_id()];
-    id_to_value_[inst->result_id()] = value_number;
-    return value_number;
+  auto value_iterator = instruction_to_value_.find(value_ins);
+  if (value_iterator != instruction_to_value_.end()) {
+    value = id_to_value_[value_iterator->first.result_id()];
+    id_to_value_[inst->result_id()] = value;
+    return value;
   }
 
   // If not, assign it a new value number.
-  uint32_t value_number = TakeNextValueNumber();
-  id_to_value_[inst->result_id()] = value_number;
-  instruction_to_value_[*inst] = value_number;
-  return value_number;
+  value = TakeNextValueNumber();
+  id_to_value_[inst->result_id()] = value;
+  instruction_to_value_[value_ins] = value;
+  return value;
+}
+
+void ValueNumberTable::BuildDominatorTreeValueNumberTable() {
+  // First value number the headers.
+  for (auto& inst : context()->annotations()) {
+    if (inst.result_id() != 0) {
+      AssignValueNumber(&inst);
+    }
+  }
+
+  for (auto& inst : context()->capabilities()) {
+    if (inst.result_id() != 0) {
+      AssignValueNumber(&inst);
+    }
+  }
+
+  for (auto& inst : context()->types_values()) {
+    if (inst.result_id() != 0) {
+      AssignValueNumber(&inst);
+    }
+  }
+
+  for (auto& inst : context()->module()->ext_inst_imports()) {
+    if (inst.result_id() != 0) {
+      AssignValueNumber(&inst);
+    }
+  }
+
+  for (ir::Function& func : *context()->module()) {
+    // For best results we want to traverse the code in reverse post order.
+    // This happens naturally because of the forward referencing rules.
+    for (ir::BasicBlock& block : func) {
+      for (ir::Instruction& inst : block) {
+        if (inst.result_id() != 0) {
+          AssignValueNumber(&inst);
+        }
+      }
+    }
+  }
 }
 
 bool ComputeSameValue::operator()(const ir::Instruction& lhs,
@@ -99,8 +206,8 @@ bool ComputeSameValue::operator()(const ir::Instruction& lhs,
 std::size_t ValueTableHash::operator()(
     const spvtools::ir::Instruction& inst) const {
   // We hash the opcode and in-operands, not the result, because we want
-  // instructions that are the same except for the result to hash to the same
-  // value.
+  // instructions that are the same except for the result to hash to the
+  // same value.
   std::u32string h;
   h.push_back(inst.opcode());
   h.push_back(inst.type_id());
index b7b987f..d7651ba 100644 (file)
@@ -37,23 +37,45 @@ class ValueTableHash {
 };
 
 // This class implements the value number analysis.  It is using a hash-based
-// approach to value numbering.  For now, it is very simple, and computes value
-// numbers for instructions when they are asked for via |GetValueNumber|.  This
-// may change in the future and should not be relied on.
+// approach to value numbering.  It is essentially doing dominator-tree value
+// numbering described in
+//
+//   Preston Briggs, Keith D. Cooper, and L. Taylor Simpson. 1997. Value
+//   numbering. Softw. Pract. Exper. 27, 6 (June 1997), 701-724.
+//   https://www.cs.rice.edu/~keith/Promo/CRPC-TR94517.pdf.gz
+//
+// The main difference is that because we do not perform redundancy elimination
+// as we build the value number table, we do not have to deal with cleaning up
+// the scope.
 class ValueNumberTable {
  public:
-  ValueNumberTable(ir::IRContext* ctx) : context_(ctx), next_value_number_(1) {}
+  ValueNumberTable(ir::IRContext* ctx) : context_(ctx), next_value_number_(1) {
+    BuildDominatorTreeValueNumberTable();
+  }
 
   // Returns the value number of the value computed by |inst|.  |inst| must have
-  // a result id that will hold the computed value.
+  // a result id that will hold the computed value.  If no value number has been
+  // assigned to the result id, then the return value is 0.
   uint32_t GetValueNumber(spvtools::ir::Instruction* inst);
 
-  // Returns the value number of the value contain in |id|.
+  // Returns the value number of the value contain in |id|.  Returns 0 if it
+  // has not been assigned a value number.
   inline uint32_t GetValueNumber(uint32_t id);
 
   ir::IRContext* context() { return context_; }
 
  private:
+  // Assigns a value number to every result id in the module.
+  void BuildDominatorTreeValueNumberTable();
+
+  // Returns the new value number.
+  uint32_t TakeNextValueNumber() { return next_value_number_++; }
+
+  // Assigns a new value number to the result of |inst| if it does not already
+  // have one.  Return the value number for |inst|.  |inst| must have a result
+  // id.
+  uint32_t AssignValueNumber(ir::Instruction* inst);
+
   std::unordered_map<spvtools::ir::Instruction, uint32_t, ValueTableHash,
                      ComputeSameValue>
       instruction_to_value_;
@@ -61,7 +83,6 @@ class ValueNumberTable {
   ir::IRContext* context_;
   uint32_t next_value_number_;
 
-  uint32_t TakeNextValueNumber() { return next_value_number_++; }
 };
 
 uint32_t ValueNumberTable::GetValueNumber(uint32_t id) {
index 54af641..4f42c81 100644 (file)
@@ -82,6 +82,36 @@ TEST_F(ValueTableTest, DifferentInstructionSameValue) {
   EXPECT_EQ(vtable.GetValueNumber(inst1), vtable.GetValueNumber(inst2));
 }
 
+TEST_F(ValueTableTest, SameValueDifferentBlock) {
+  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
+          %3 = OpTypeVoid
+          %4 = OpTypeFunction %3
+          %5 = OpTypeFloat 32
+          %6 = OpTypePointer Function %5
+          %2 = OpFunction %3 None %4
+          %7 = OpLabel
+          %8 = OpVariable %6 Function
+          %9 = OpLoad %5 %8
+         %10 = OpFAdd %5 %9 %9
+               OpBranch %11
+         %11 = OpLabel
+         %12 = OpFAdd %5 %9 %9
+               OpReturn
+               OpFunctionEnd
+  )";
+  auto context = BuildModule(SPV_ENV_UNIVERSAL_1_2, nullptr, text);
+  opt::ValueNumberTable vtable(context.get());
+  ir::Instruction* inst1 = context->get_def_use_mgr()->GetDef(10);
+  ir::Instruction* inst2 = context->get_def_use_mgr()->GetDef(12);
+  EXPECT_EQ(vtable.GetValueNumber(inst1), vtable.GetValueNumber(inst2));
+}
+
 TEST_F(ValueTableTest, DifferentValue) {
   const std::string text = R"(
                OpCapability Shader
@@ -110,6 +140,36 @@ TEST_F(ValueTableTest, DifferentValue) {
   EXPECT_NE(vtable.GetValueNumber(inst1), vtable.GetValueNumber(inst2));
 }
 
+TEST_F(ValueTableTest, DifferentValueDifferentBlock) {
+  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
+          %3 = OpTypeVoid
+          %4 = OpTypeFunction %3
+          %5 = OpTypeFloat 32
+          %6 = OpTypePointer Function %5
+          %2 = OpFunction %3 None %4
+          %7 = OpLabel
+          %8 = OpVariable %6 Function
+          %9 = OpLoad %5 %8
+         %10 = OpFAdd %5 %9 %9
+               OpBranch %11
+         %11 = OpLabel
+         %12 = OpFAdd %5 %9 %10
+               OpReturn
+               OpFunctionEnd
+  )";
+  auto context = BuildModule(SPV_ENV_UNIVERSAL_1_2, nullptr, text);
+  opt::ValueNumberTable vtable(context.get());
+  ir::Instruction* inst1 = context->get_def_use_mgr()->GetDef(10);
+  ir::Instruction* inst2 = context->get_def_use_mgr()->GetDef(12);
+  EXPECT_NE(vtable.GetValueNumber(inst1), vtable.GetValueNumber(inst2));
+}
+
 TEST_F(ValueTableTest, SameLoad) {
   const std::string text = R"(
                OpCapability Shader
@@ -367,4 +427,162 @@ TEST_F(ValueTableTest, DifferentTypes) {
   ir::Instruction* inst2 = context->get_def_use_mgr()->GetDef(12);
   EXPECT_NE(vtable.GetValueNumber(inst1), vtable.GetValueNumber(inst2));
 }
+
+TEST_F(ValueTableTest, CopyObject) {
+  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
+          %3 = OpTypeVoid
+          %4 = OpTypeFunction %3
+          %5 = OpTypeFloat 32
+          %6 = OpTypePointer Function %5
+          %2 = OpFunction %3 None %4
+          %7 = OpLabel
+          %8 = OpVariable %6 Function
+          %9 = OpLoad %5 %8
+         %10 = OpCopyObject %5 %9
+               OpReturn
+               OpFunctionEnd
+  )";
+  auto context = BuildModule(SPV_ENV_UNIVERSAL_1_2, nullptr, text);
+  opt::ValueNumberTable vtable(context.get());
+  ir::Instruction* inst1 = context->get_def_use_mgr()->GetDef(9);
+  ir::Instruction* inst2 = context->get_def_use_mgr()->GetDef(10);
+  EXPECT_EQ(vtable.GetValueNumber(inst1), vtable.GetValueNumber(inst2));
+}
+
+// Test that a phi where the operands have the same value assigned that value
+// to the result of the phi.
+TEST_F(ValueTableTest, PhiTest1) {
+  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
+          %3 = OpTypeVoid
+          %4 = OpTypeFunction %3
+          %5 = OpTypeFloat 32
+          %6 = OpTypePointer Uniform %5
+          %7 = OpTypeBool
+          %8 = OpConstantTrue %7
+          %9 = OpVariable %6 Uniform
+          %2 = OpFunction %3 None %4
+         %10 = OpLabel
+               OpBranchConditional %8 %11 %12
+         %11 = OpLabel
+         %13 = OpLoad %5 %9
+               OpBranch %14
+         %12 = OpLabel
+         %15 = OpLoad %5 %9
+               OpBranch %14
+         %14 = OpLabel
+         %16 = OpPhi %5 %13 %11 %15 %12
+               OpReturn
+               OpFunctionEnd
+  )";
+  auto context = BuildModule(SPV_ENV_UNIVERSAL_1_2, nullptr, text);
+  opt::ValueNumberTable vtable(context.get());
+  ir::Instruction* inst1 = context->get_def_use_mgr()->GetDef(13);
+  ir::Instruction* inst2 = context->get_def_use_mgr()->GetDef(15);
+  ir::Instruction* phi = context->get_def_use_mgr()->GetDef(16);
+  EXPECT_EQ(vtable.GetValueNumber(inst1), vtable.GetValueNumber(inst2));
+  EXPECT_EQ(vtable.GetValueNumber(inst1), vtable.GetValueNumber(phi));
+}
+
+// When the values for the inputs to a phi do not match, then the phi should
+// have its own value number.
+TEST_F(ValueTableTest, PhiTest2) {
+  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
+          %3 = OpTypeVoid
+          %4 = OpTypeFunction %3
+          %5 = OpTypeFloat 32
+          %6 = OpTypePointer Uniform %5
+          %7 = OpTypeBool
+          %8 = OpConstantTrue %7
+          %9 = OpVariable %6 Uniform
+         %10 = OpVariable %6 Uniform
+          %2 = OpFunction %3 None %4
+         %11 = OpLabel
+               OpBranchConditional %8 %12 %13
+         %12 = OpLabel
+         %14 = OpLoad %5 %9
+               OpBranch %15
+         %13 = OpLabel
+         %16 = OpLoad %5 %10
+               OpBranch %15
+         %15 = OpLabel
+         %17 = OpPhi %14 %12 %16 %13
+               OpReturn
+               OpFunctionEnd
+  )";
+  auto context = BuildModule(SPV_ENV_UNIVERSAL_1_2, nullptr, text);
+  opt::ValueNumberTable vtable(context.get());
+  ir::Instruction* inst1 = context->get_def_use_mgr()->GetDef(14);
+  ir::Instruction* inst2 = context->get_def_use_mgr()->GetDef(16);
+  ir::Instruction* phi = context->get_def_use_mgr()->GetDef(17);
+  EXPECT_NE(vtable.GetValueNumber(inst1), vtable.GetValueNumber(inst2));
+  EXPECT_NE(vtable.GetValueNumber(inst1), vtable.GetValueNumber(phi));
+  EXPECT_NE(vtable.GetValueNumber(inst2), vtable.GetValueNumber(phi));
+}
+
+// Test that a phi node in a loop header gets a new value because one of its
+// inputs comes from later in the loop.
+TEST_F(ValueTableTest, PhiLoopTest) {
+  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
+          %3 = OpTypeVoid
+          %4 = OpTypeFunction %3
+          %5 = OpTypeFloat 32
+          %6 = OpTypePointer Uniform %5
+          %7 = OpTypeBool
+          %8 = OpConstantTrue %7
+          %9 = OpVariable %6 Uniform
+         %10 = OpVariable %6 Uniform
+          %2 = OpFunction %3 None %4
+         %11 = OpLabel
+         %12 = OpLoad %5 %9
+               OpSelectionMerge %13 None
+               OpBranchConditional %8 %14 %13
+         %14 = OpLabel
+         %15 = OpPhi %5 %12 %11 %16 %14
+         %16 = OpLoad %5 %9
+               OpLoopMerge %17 %14 None
+               OpBranchConditional %8 %14 %17
+         %17 = OpLabel
+               OpBranch %13
+         %13 = OpLabel
+         %18 = OpPhi %5 %12 %11 %16 %17
+               OpReturn
+               OpFunctionEnd
+  )";
+  auto context = BuildModule(SPV_ENV_UNIVERSAL_1_2, nullptr, text);
+  opt::ValueNumberTable vtable(context.get());
+  ir::Instruction* inst1 = context->get_def_use_mgr()->GetDef(12);
+  ir::Instruction* inst2 = context->get_def_use_mgr()->GetDef(16);
+  EXPECT_EQ(vtable.GetValueNumber(inst1), vtable.GetValueNumber(inst2));
+
+  ir::Instruction* phi1 = context->get_def_use_mgr()->GetDef(15);
+  EXPECT_NE(vtable.GetValueNumber(inst1), vtable.GetValueNumber(phi1));
+
+  ir::Instruction* phi2 = context->get_def_use_mgr()->GetDef(18);
+  EXPECT_EQ(vtable.GetValueNumber(inst1), vtable.GetValueNumber(phi2));
+  EXPECT_NE(vtable.GetValueNumber(phi1), vtable.GetValueNumber(phi2));
+}
 }  // anonymous namespace