Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more tests and KillInst() to DefUseManager #347

Closed
wants to merge 4 commits into from
Closed

Add more tests and KillInst() to DefUseManager #347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
266b2ed
Replace the SetOperand() with SetInOperand() and SetResultType()
Qining Aug 16, 2016
d28a5d8
Add more tests and KillInst() to DefUseManager
Qining Aug 12, 2016
3daffcf
Update and fix blurbs.
Qining Aug 15, 2016
db5e0d9
draft
Qining Aug 15, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 88 additions & 39 deletions source/opt/def_use_manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,33 +37,32 @@ namespace opt {
namespace analysis {

void DefUseManager::AnalyzeDefUse(ir::Module* module) {
Reset();
module->ForEachInst(std::bind(&DefUseManager::AnalyzeInstDefUse, this,
std::placeholders::_1));
}

void DefUseManager::AnalyzeInstDefUse(ir::Instruction* inst) {
const uint32_t def_id = inst->result_id();
// Clear the records of def_id first if it has been recorded before.
ClearDef(def_id);
// Clear the records of the given instruction if it has been analyzed before.
ClearInst(inst);

if (def_id != 0) id_to_def_[def_id] = inst;
const uint32_t def_id = inst->result_id();
if (def_id != 0) {
// If the new instruction defines an existing result id, clear the records
// of the existing id first.
// ClearDef(def_id);
id_to_def_[def_id] = inst;
}

for (uint32_t i = 0; i < inst->NumOperands(); ++i) {
switch (inst->GetOperand(i).type) {
// For any id type but result id type
case SPV_OPERAND_TYPE_ID:
case SPV_OPERAND_TYPE_TYPE_ID:
case SPV_OPERAND_TYPE_MEMORY_SEMANTICS_ID:
case SPV_OPERAND_TYPE_SCOPE_ID: {
uint32_t use_id = inst->GetSingleWordOperand(i);
// use_id is used by the instruction generating def_id.
id_to_uses_[use_id].push_back({inst, i});
if (def_id != 0) result_id_to_used_ids_[def_id].push_back(use_id);
} break;
default:
break;
if (IsIdUse(inst->GetOperand(i))) {
uint32_t use_id = inst->GetSingleWordOperand(i);
// use_id is used by this instruction.
id_to_uses_[use_id].push_back({inst, i});
}
}

analyzed_insts_.insert(inst);
}

ir::Instruction* DefUseManager::GetDef(uint32_t id) {
Expand All @@ -78,21 +77,33 @@ UseList* DefUseManager::GetUses(uint32_t id) {

bool DefUseManager::KillDef(uint32_t id) {
if (id_to_def_.count(id) == 0) return false;

ir::Instruction* defining_inst = id_to_def_.at(id);
ClearDef(id);
defining_inst->ToNop();
KillInst(id_to_def_[id]);
return true;
}

void DefUseManager::KillInst(ir::Instruction* inst) {
if (!inst) return;
ClearInst(inst);
inst->ToNop();
}

bool DefUseManager::ReplaceAllUsesWith(uint32_t before, uint32_t after) {
if (before == after) return false;
if (id_to_uses_.count(before) == 0) return false;

for (auto it = id_to_uses_[before].cbegin(); it != id_to_uses_[before].cend();
++it) {
// Make the modification in the instruction.
it->inst->SetOperand(it->operand_index, {after});
uint32_t type_result_id_count =
(it->inst->result_id() != 0) + (it->inst->type_id() != 0);
if (it->inst->type_id() != 0 && it->operand_index == 0) {
it->inst->SetResultType(after);
} else {
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;
// Make the modification in the instruction.
it->inst->SetInOperand(in_operand_pos, {after});
}
// Register the use of |after| id into id_to_uses_.
// TODO(antiagainst): de-duplication.
id_to_uses_[after].push_back({it->inst, it->operand_index});
Expand All @@ -102,25 +113,63 @@ bool DefUseManager::ReplaceAllUsesWith(uint32_t before, uint32_t after) {
}

void DefUseManager::ClearDef(uint32_t def_id) {
if (id_to_def_.count(def_id) == 0) return;

// Go through all ids used by this instruction, remove this instruction's
// uses of them.
for (const auto use_id : result_id_to_used_ids_[def_id]) {
if (id_to_uses_.count(use_id) == 0) continue;
auto& uses = id_to_uses_[use_id];
for (auto it = uses.begin(); it != uses.end();) {
if (it->inst->result_id() == def_id) {
it = uses.erase(it);
} else {
++it;
const auto& iter = id_to_def_.find(def_id);
if (iter == id_to_def_.end()) return;
EraseInstUsesOfOperands(*iter->second);
// If a result id is defined by this instruction, remove the use records of
// the id.
id_to_uses_.erase(def_id); // Remove all uses of this id.
id_to_def_.erase(def_id);
}

void DefUseManager::ClearInst(ir::Instruction* inst) {
// Do nothing if the instruction is a nullptr or it has not been analyzed
// before.
if (!inst) return;

EraseInstUsesOfOperands(*inst);
// If a result id is defined by this instruction, remove the use records of
// the id.
if (inst->result_id() != 0) {
// assert(id_to_def_[inst->result_id()] == inst);
id_to_uses_.erase(inst->result_id()); // Remove all uses of this id.
id_to_def_.erase(inst->result_id());
}
}

bool DefUseManager::IsIdUse(const ir::Operand& operand) const {
switch (operand.type) {
// For any id type but result id type
case SPV_OPERAND_TYPE_ID:
case SPV_OPERAND_TYPE_TYPE_ID:
case SPV_OPERAND_TYPE_MEMORY_SEMANTICS_ID:
case SPV_OPERAND_TYPE_SCOPE_ID:
return true;
default:
return false;
}
}

void DefUseManager::EraseInstUsesOfOperands(const ir::Instruction& inst) {
// Go through all ids, except the result id, used by this instruction, remove
// this instruction's uses of those ids.
for (uint32_t i = 0; i < inst.NumOperands(); i++) {
if (IsIdUse(inst.GetOperand(i))) {
uint32_t operand_id = inst.GetSingleWordOperand(i);
auto iter = id_to_uses_.find(operand_id);
if (iter != id_to_uses_.end()) {
auto& uses = iter->second;
for (auto it = uses.begin(); it != uses.end();) {
if (it->inst == &inst) {
it = uses.erase(it);
} else {
++it;
}
}
if (uses.empty()) id_to_uses_.erase(operand_id);
}
}
if (uses.empty()) id_to_uses_.erase(use_id);
}
result_id_to_used_ids_.erase(def_id);
id_to_uses_.erase(def_id); // Remove all uses of this id.
id_to_def_.erase(def_id);
}

} // namespace analysis
Expand Down
53 changes: 40 additions & 13 deletions source/opt/def_use_manager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

#include <list>
#include <unordered_map>
#include <unordered_set>
#include <utility>
#include <vector>

Expand Down Expand Up @@ -57,13 +58,15 @@ class DefUseManager {
using IdToDefMap = std::unordered_map<uint32_t, ir::Instruction*>;
using IdToUsesMap = std::unordered_map<uint32_t, UseList>;

// Analyzes the defs and uses in the given |module| and populates data
// structures in this class.
// Reset this manager and analyzes the defs and uses in the given |module|
// and populates data structures in this class.
// TODO(antiagainst): This method should not modify the given module. Create
// const overload for ForEachInst().
void AnalyzeDefUse(ir::Module* module);

// Analyzes the defs and uses in the given |inst|.
// Analyzes the defs and uses in the given |inst|. If |inst| has been
// analyzed by this manager before, the existing records will be overwritten
// by the latest analysis result.
void AnalyzeInstDefUse(ir::Instruction* inst);

// Returns the def instruction for the given |id|. If there is no instruction
Expand All @@ -84,27 +87,51 @@ class DefUseManager {
// information kept in this manager, but not the operands in the original
// instructions.
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| using |inst|'s operand ids.
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
// |before| id. If |after| is the same as |before|, does nothing and returns
// false.
bool ReplaceAllUsesWith(uint32_t before, uint32_t after);

private:
// 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.
// Clears the internal def-use records of a defined id if the given |def_id|
// is recorded by this manager. Does nothing if |def_id| has not been
// recorded yet. 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.
void ClearDef(uint32_t def_id);

using ResultIdToUsedIdsMap =
std::unordered_map<uint32_t, std::vector<uint32_t>>;

// Clears the internal def-use records of the given instruction |inst| if it
// has been analyzed by this manager. The use information of its operand ids
// will be updated: "the record: |inst| uses |operand id| will be removed".
// If the instruction is defining a result id, the uses of the result id will
// also be removed. Note that if |inst| has not been analyzed before, this
// function does nothing even though |inst| may define an existing result id.
void ClearInst(ir::Instruction* inst);

// Returns true if the operand's id is an use.
bool IsIdUse(const ir::Operand& operand) const;

// Erases the records that instruction |inst| uses the ids in its operands.
// Does nothing if all the operands are not ids or the instruction does not
// have operands.
void EraseInstUsesOfOperands(const ir::Instruction& inst);

// Resets the internal records
void Reset() {
analyzed_insts_.clear();
id_to_def_.clear();
id_to_uses_.clear();
}

std::unordered_set<ir::Instruction*>
analyzed_insts_; // Analyzed instructions
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need this map at all. It just guards clearing of the def+uses of an instruction. But the clear method should be resilient to the case where the instruction hasn't been analyzed before.
?!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There may be four cases (depends on whether we want to make ir::Instruction to be immutable later) we need to differentiate:

  1. a new instruction without result_id but with already analyzed operands.
  2. a new instruction with result_id and also has its operands analyzed.
  3. an analyzed but mutated instruction, the instruction does have a result id.
  4. an analyzed but mutated instruction, the instruction does not have a result id.

For case 1), the desired result is that, the def-use records of the analyzed operands should be +1 to have the new added instruction as their new uses.

For case 2), the desired result is that, the original result_id defining instruction should be cleared first, then the new added instruction can update the records for the involved operands. This is a -1 followed by +1 for the operands.

For case 3), we should -1 first then +1, same as a new instruction with existing result id.

For case 4), we should also -1 first then +1, same as a new instruction with existing result id, but this is different with case 1).

To differentiate the four cases we have two options:

a) See if the instruction has a result id.
If the instruction has a result id, clear internal records for the id, then analyze the new instruction. Otherwise analyze the new instruction directly. But this way we can not differentiate case 1 and case 4. We will -1 for the operands in case 1.

b) Use a set to hold all the analyzed instructions.
If the instruction has been analyzed, clear its record first.
If the result id has been analyzed before, clear its def_id record also.
Add record for the given instruction as if it is a new instruction.

Actually the fundamental solution to this hassle is to use instruction* instead of id(uint32_t) for the internal maps then we can unify the behavior. But that may changed the structure a lot. This is the reason why we need such a set here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My point was that asking to remove a use from an instruction should be a no-op if that use is not actually recorded. Then your cases 1 and 4 don't need to be distinguished.

But I see there is complexity in having instruction's operands already be analyzed from the definer's perspective, and then you try to analyze the user's perspective. I have a headache and I'd like a second opinion on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, yes, you are right. If the target instruction is really a new instruction, ClearInst() won't change the def-use records of the target instruction's operand id.

I missed the "is same instruction" check when erasing use records.

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.
ResultIdToUsedIdsMap result_id_to_used_ids_;
};

} // namespace analysis
Expand Down
22 changes: 16 additions & 6 deletions source/opt/instruction.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,10 @@ class Instruction {
// not expected to be used with logical operands consisting of multiple SPIR-V
// words.
uint32_t GetSingleWordOperand(uint32_t index) const;
// Sets the |index|-th operand's data to the given |data|.
inline void SetOperand(uint32_t index, std::vector<uint32_t>&& data);
// Sets the |index|-th in-operand's data to the given |data|.
inline void SetInOperand(uint32_t index, std::vector<uint32_t>&& data);
// Sets the result type id.
inline void SetResultType(uint32_t ty_id);

// The following methods are similar to the above, but are for in operands.
uint32_t NumInOperands() const {
Expand Down Expand Up @@ -179,10 +181,18 @@ inline const Operand& Instruction::GetOperand(uint32_t index) const {
return operands_[index];
};

inline void Instruction::SetOperand(uint32_t index,
std::vector<uint32_t>&& data) {
assert(index < operands_.size() && "operand index out of bound");
operands_[index].words = std::move(data);
inline void Instruction::SetInOperand(uint32_t index,
std::vector<uint32_t>&& data) {
assert(index + TypeResultIdCount() < operands_.size() &&
"operand index out of bound");
operands_[index + TypeResultIdCount()].words = std::move(data);
}

inline void Instruction::SetResultType(uint32_t ty_id) {
if (type_id_ != 0) {
type_id_ = ty_id;
operands_[0] = {SPV_OPERAND_TYPE_TYPE_ID, {ty_id}};
}
}

inline bool Instruction::IsNop() const {
Expand Down
Loading