Skip to content

Commit

Permalink
Add test for "analyzing new inst with existing module";
Browse files Browse the repository at this point in the history 
Add blurbs;
Remove ClearDef() function;
Fixed a bug in DefUseManager::ReplaceAllUsesWith() that OpName
instruction may trigger the assertion incorrectly.
update the blurbs for EraseUseRecordsOfOperandIds()
  • Loading branch information
@Qining
Qining committed Aug 18, 2016
1 parent 2ee8a12 commit 0cbc48b
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 57 deletions.
58 changes: 36 additions & 22 deletions source/opt/def_use_manager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,21 @@ namespace analysis {
void DefUseManager::AnalyzeInstDefUse(ir::Instruction* inst) {
const uint32_t def_id = inst->result_id();
if (def_id != 0) {
ClearDef(def_id);
auto iter = id_to_def_.find(def_id);
if (iter != id_to_def_.end()) {
// Clear the original instruction that defining the same result id of the
// new instruction.
ClearInst(iter->second);
}
id_to_def_[def_id] = inst;
} else {
ClearInst(inst);
}

inst_to_used_ids_[inst].reserve(inst->NumOperands());
// Create entry for the given instruction. Note that the instruction may
// not have any in-operands. In such cases, we still need a entry for those
// instructions so this manager knows it has seen the instruction later.
inst_to_used_ids_[inst] = {};

for (uint32_t i = 0; i < inst->NumOperands(); ++i) {
switch (inst->GetOperand(i).type) {
Expand All @@ -66,18 +74,21 @@ void DefUseManager::AnalyzeInstDefUse(ir::Instruction* inst) {
}

ir::Instruction* DefUseManager::GetDef(uint32_t id) {
if (id_to_def_.count(id) == 0) return nullptr;
return id_to_def_.at(id);
auto iter = id_to_def_.find(id);
if (iter == id_to_def_.end()) return nullptr;
return iter->second;
}

UseList* DefUseManager::GetUses(uint32_t id) {
if (id_to_uses_.count(id) == 0) return nullptr;
return &id_to_uses_.at(id);
auto iter = id_to_uses_.find(id);
if (iter == id_to_uses_.end()) return nullptr;
return &iter->second;
}

bool DefUseManager::KillDef(uint32_t id) {
if (id_to_def_.count(id) == 0) return false;
KillInst(id_to_def_[id]);
auto iter = id_to_def_.find(id);
if (iter == id_to_def_.end()) return false;
KillInst(iter->second);
return true;
}

Expand All @@ -95,12 +106,21 @@ bool DefUseManager::ReplaceAllUsesWith(uint32_t before, uint32_t after) {
++it) {
const uint32_t type_result_id_count =
(it->inst->result_id() != 0) + (it->inst->type_id() != 0);
if (!it->operand_index) {
assert(it->inst->type_id() != 0 &&
"result type id considered as using while the instruction doesn't "
"have a result type id");
it->inst->SetResultType(after);

if (it->operand_index < type_result_id_count) {
// Update the type_id. Note that result id is immutable so it should
// never be updated.
if (it->inst->type_id() != 0 && it->operand_index == 0) {
it->inst->SetResultType(after);
} else if (it->inst->type_id() == 0) {
assert(false &&
"Result type id considered as using while the instruction "
"doesn't have a result type id.");
} else {
assert(false && "Trying Setting the result id which is immutable.");
}
} else {
// Update an in-operand.
assert(it->operand_index >= type_result_id_count &&
"the operand to be set is not an in-operand.");
uint32_t in_operand_pos = it->operand_index - type_result_id_count;
Expand All @@ -121,13 +141,6 @@ void DefUseManager::AnalyzeDefUse(ir::Module* module) {
std::placeholders::_1));
}

void DefUseManager::ClearDef(uint32_t def_id) {
auto iter = id_to_def_.find(def_id);
if (iter != id_to_def_.end()) {
ClearInst(iter->second);
}
}

void DefUseManager::ClearInst(ir::Instruction* inst) {
auto iter = inst_to_used_ids_.find(inst);
if (iter != inst_to_used_ids_.end()) {
Expand All @@ -145,8 +158,9 @@ void DefUseManager::EraseUseRecordsOfOperandIds(const ir::Instruction* inst) {
auto iter = inst_to_used_ids_.find(inst);
if (iter != inst_to_used_ids_.end()) {
for (const auto use_id : iter->second) {
if (id_to_uses_.count(use_id) == 0) continue;
auto& uses = id_to_uses_[use_id];
auto uses_iter = id_to_uses_.find(use_id);
if (uses_iter == id_to_uses_.end()) continue;
auto& uses = uses_iter->second;
for (auto it = uses.begin(); it != uses.end();) {
if (it->inst == inst) {
it = uses.erase(it);
Expand Down
34 changes: 15 additions & 19 deletions source/opt/def_use_manager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,12 @@ class DefUseManager {

// Turns the instruction defining the given |id| into a Nop. Returns true on
// success, false if the given |id| is not defined at all. This method also
// erases both the uses of |id| and the |id|-generating instruction's use
// information kept in this manager, but not the operands in the original
// instructions.
// erases both the uses of |id| and the information of this |id|-generating
// instruction's uses of its operands.
bool KillDef(uint32_t id);
// Turns the given instruction |inst| to a Nop, also erases both the use
// records of the result id of |inst| (if any) and the corresponding use
// records of: |inst| useing |inst|'s operand ids.
// Turns the given instruction |inst| to a Nop. This method erases the
// information of the given instruction's uses of its operands. If |inst|
// defines an result id, the uses of the result id will also be erased.
void KillInst(ir::Instruction* inst);
// Replaces all uses of |before| id with |after| id. Returns true if any
// replacement happens. This method does not kill the definition of the
Expand All @@ -95,34 +94,31 @@ class DefUseManager {
bool ReplaceAllUsesWith(uint32_t before, uint32_t after);

private:
using InstToUsedIdsMap =
std::unordered_map<const ir::Instruction*, std::vector<uint32_t>>;

// Analyzes the defs and uses in the given |module| and populates data
// structures in this class. Does nothing if |module| is nullptr.
void AnalyzeDefUse(ir::Module* module);

// Clear the internal def-use record of a defined id if the given |def_id| is
// recorded by this manager. This method will erase both the uses of |def_id|
// and the |def_id|-generating instruction's use information kept in this
// manager, but not the operands in the original instructions. Does nothing
// if |def_id| was not recorded before.
void ClearDef(uint32_t def_id);

// Clear the internal def-use record of the given instruction |inst|. This
// method will update the use information of the operand ids of |inst|. The
// record: |inst| uses an |id|, will be removed from the use records of |id|.
// If |inst| defines an result id, the use record of this result id will also
// be removed. Does nothing if |inst| was not analyzed before.
void ClearInst(ir::Instruction* inst);

// Erases the records that a given instruction |inst| uses its operand ids.
// Erases the records that a given instruction uses its operand ids.
// This method updates the id_to_uses_ map with the operand ids of the
// given instruction and remove the use records of the given instruction from
// the records of its operand ids. It also erase the mapping from the given
// instruction to its used operand ids in the inst_to_used_ids_ map.
void EraseUseRecordsOfOperandIds(const ir::Instruction* inst);

using InstToUsedIdsMap =
std::unordered_map<const ir::Instruction*, std::vector<uint32_t>>;

IdToDefMap id_to_def_; // Mapping from ids to their definitions
IdToUsesMap id_to_uses_; // Mapping from ids to their uses
// Mapping from result ids to the ids used in the instructions generating the
// result ids.
// Mapping from instructions to the ids used in the instructions generating
// the result ids.
InstToUsedIdsMap inst_to_used_ids_;
};

Expand Down
79 changes: 63 additions & 16 deletions test/opt/test_def_use.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1153,7 +1153,7 @@ TEST(DefUseTest, OpSwitch) {
}

// Creates an |result_id| = OpTypeInt 32 1 instruction.
ir::Instruction OpTypeIntInstruction(uint32_t result_id) {
ir::Instruction Int32TypeInstruction(uint32_t result_id) {
return ir::Instruction(SpvOp::SpvOpTypeInt, 0, result_id,
{ir::Operand(SPV_OPERAND_TYPE_LITERAL_INTEGER, {32}),
ir::Operand(SPV_OPERAND_TYPE_LITERAL_INTEGER, {1})});
Expand All @@ -1180,9 +1180,13 @@ ir::Instruction BranchInstruction(uint32_t target_id) {
});
}

// Test case for analyzing individual instructions.
struct AnalyzeInstDefUseTestCase {
std::vector<ir::Instruction> insts; // instrutions to be analyzed in order.
InstDefUse expected_define_use; // expected analysis result.
const char*
module_text; // If not empty, the module will be analyzed before above
// instrcutions.
InstDefUse expected_define_use;
};

using AnalyzeInstDefUseTest =
Expand All @@ -1192,8 +1196,13 @@ using AnalyzeInstDefUseTest =
TEST_P(AnalyzeInstDefUseTest, Case) {
auto tc = GetParam();

// Build module.
std::unique_ptr<ir::Module> module =
SpvTools(SPV_ENV_UNIVERSAL_1_1).BuildModule(tc.module_text);
ASSERT_NE(nullptr, module);

// Analyze the instructions.
opt::analysis::DefUseManager manager(nullptr);
opt::analysis::DefUseManager manager(module.get());
for (ir::Instruction& inst : tc.insts) {
manager.AnalyzeInstDefUse(&inst);
}
Expand All @@ -1207,7 +1216,8 @@ INSTANTIATE_TEST_CASE_P(
TestCase, AnalyzeInstDefUseTest,
::testing::ValuesIn(std::vector<AnalyzeInstDefUseTestCase>{
{ // A type declaring instruction.
{OpTypeIntInstruction(1)},
{Int32TypeInstruction(1)},
"",
{
// defs
{{1, "%1 = OpTypeInt 32 1"}},
Expand All @@ -1216,16 +1226,17 @@ INSTANTIATE_TEST_CASE_P(
},
{ // A type declaring instruction and a constant value.
{
OpTypeIntInstruction(1),
Int32TypeInstruction(1),
ConstantBoolInstruction(true, 1, 2),
},
"",
{
{ // defs
{1, "%1 = OpTypeInt 32 1"},
{2, "%2 = OpConstantTrue %1"},
{2, "%2 = OpConstantTrue %1"}, // It is fine the SPIR-V code here is invalid.
},
{ // uses
{1,{"%2 = OpConstantTrue %1"}},
{1, {"%2 = OpConstantTrue %1"}},
},
},
},
Expand All @@ -1238,6 +1249,7 @@ INSTANTIATE_TEST_CASE_P(
// records of the above one.
ConstantBoolInstruction(false, 3, 2),
},
"",
{
// defs
{{2, "%2 = OpConstantFalse %3"}},
Expand All @@ -1251,19 +1263,53 @@ INSTANTIATE_TEST_CASE_P(
BranchInstruction(2),
LabelInstruction(2),
},
"",
{
// defs
{{2, "%2 = OpLabel"}},
// uses
{{2, {"OpBranch %2"}}},
}
}
},
{ // Analyzing an additional instruction with new result id to an
// existing module.
{
ConstantBoolInstruction(true, 1, 2),
},
"%1 = OpTypeInt 32 1 ",
{
{ // defs
{1, "%1 = OpTypeInt 32 1"},
{2, "%2 = OpConstantTrue %1"},
},
{ // uses
{1, {"%2 = OpConstantTrue %1"}},
},
}
},
{ // Analyzing an additional instruction with existing result id to an
// existing module.
{
ConstantBoolInstruction(true, 1, 2),
},
"%1 = OpTypeInt 32 1 "
"%2 = OpTypeBool ",
{
{ // defs
{1, "%1 = OpTypeInt 32 1"},
{2, "%2 = OpConstantTrue %1"},
},
{ // uses
{1, {"%2 = OpConstantTrue %1"}},
},
}
},
}));
// clang-format on

struct KillInstTestCase {
const char* before;
std::unordered_set<uint32_t> indices_inst_to_kill;
std::unordered_set<uint32_t> indices_for_inst_to_kill;
const char* after;
InstDefUse expected_define_use;
};
Expand All @@ -1282,7 +1328,7 @@ TEST_P(KillInstTest, Case) {
uint32_t index = 0;
opt::analysis::DefUseManager manager(module.get());
module->ForEachInst([&index, &tc, &manager](ir::Instruction* inst) {
if (tc.indices_inst_to_kill.count(index) != 0) {
if (tc.indices_for_inst_to_kill.count(index) != 0) {
manager.KillInst(inst);
}
index++;
Expand All @@ -1309,9 +1355,9 @@ INSTANTIATE_TEST_CASE_P(
"%7 = OpLabel "
" OpReturn "
" OpFunctionEnd",
{0, 1, 3, 5, 7},
"OpNop\n"
{0, 3, 5, 7},
"OpNop\n"
"%4 = OpLabel\n"
"OpBranch %5\n"
"OpNop\n"
"OpBranch %6\n"
Expand All @@ -1322,9 +1368,9 @@ INSTANTIATE_TEST_CASE_P(
"OpFunctionEnd",
{
// no defs
{},
{{4, "%4 = OpLabel"}},
// no uses
{}
{{4, {"OpBranch %4"}}}
}
},
// Kill instructions that do not have result ids.
Expand All @@ -1339,14 +1385,14 @@ INSTANTIATE_TEST_CASE_P(
"%7 = OpLabel "
" OpReturn "
" OpFunctionEnd",
{2, 4, 6},
{2, 4},
"%2 = OpFunction %1 None %3\n"
"%4 = OpLabel\n"
"OpNop\n"
"%5 = OpLabel\n"
"OpNop\n"
"%6 = OpLabel\n"
"OpNop\n"
"OpBranch %4\n"
"%7 = OpLabel\n"
"OpReturn\n"
"OpFunctionEnd",
Expand All @@ -1363,6 +1409,7 @@ INSTANTIATE_TEST_CASE_P(
{
{1, {"%2 = OpFunction %1 None %3"}},
{3, {"%2 = OpFunction %1 None %3"}},
{4, {"OpBranch %4"}},
}
}
},
Expand Down

0 comments on commit 0cbc48b

Please sign in to comment.