Use a struct (instead of tuple), with explicit member names. (#4621)

* Cleanup includes.

* Simplify assertion.

* Use a struct with named members for 'UserEntry'
This commit is contained in:
Sebastien Alaiwan 2021-12-09 15:40:29 +01:00 committed by GitHub
parent 438096e0c2
commit f37551d2b6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 25 additions and 30 deletions

View File

@ -14,11 +14,6 @@
#include "source/opt/def_use_manager.h"
#include <iostream>
#include "source/opt/log.h"
#include "source/opt/reflect.h"
namespace spvtools {
namespace opt {
namespace analysis {
@ -58,8 +53,8 @@ void DefUseManager::AnalyzeInstUse(Instruction* inst) {
case SPV_OPERAND_TYPE_SCOPE_ID: {
uint32_t use_id = inst->GetSingleWordOperand(i);
Instruction* def = GetDef(use_id);
if (!def) assert(false && "Definition is not registered.");
id_to_users_.insert(UserEntry(def, inst));
assert(def && "Definition is not registered.");
id_to_users_.insert(UserEntry{def, inst});
used_ids->push_back(use_id);
} break;
default:
@ -102,13 +97,13 @@ const Instruction* DefUseManager::GetDef(uint32_t id) const {
DefUseManager::IdToUsersMap::const_iterator DefUseManager::UsersBegin(
const Instruction* def) const {
return id_to_users_.lower_bound(
UserEntry(const_cast<Instruction*>(def), nullptr));
UserEntry{const_cast<Instruction*>(def), nullptr});
}
bool DefUseManager::UsersNotEnd(const IdToUsersMap::const_iterator& iter,
const IdToUsersMap::const_iterator& cached_end,
const Instruction* inst) const {
return (iter != cached_end && iter->first == inst);
return (iter != cached_end && iter->def == inst);
}
bool DefUseManager::UsersNotEnd(const IdToUsersMap::const_iterator& iter,
@ -125,7 +120,7 @@ bool DefUseManager::WhileEachUser(
auto end = id_to_users_.end();
for (auto iter = UsersBegin(def); UsersNotEnd(iter, end, def); ++iter) {
if (!f(iter->second)) return false;
if (!f(iter->user)) return false;
}
return true;
}
@ -158,7 +153,7 @@ bool DefUseManager::WhileEachUse(
auto end = id_to_users_.end();
for (auto iter = UsersBegin(def); UsersNotEnd(iter, end, def); ++iter) {
Instruction* user = iter->second;
Instruction* user = iter->user;
for (uint32_t idx = 0; idx != user->NumOperands(); ++idx) {
const Operand& op = user->GetOperand(idx);
if (op.type != SPV_OPERAND_TYPE_RESULT_ID && spvIsIdType(op.type)) {
@ -258,7 +253,7 @@ void DefUseManager::EraseUseRecordsOfOperandIds(const Instruction* inst) {
if (iter != inst_to_used_ids_.end()) {
for (auto use_id : iter->second) {
id_to_users_.erase(
UserEntry(GetDef(use_id), const_cast<Instruction*>(inst)));
UserEntry{GetDef(use_id), const_cast<Instruction*>(inst)});
}
inst_to_used_ids_.erase(iter);
}

View File

@ -15,10 +15,8 @@
#ifndef SOURCE_OPT_DEF_USE_MANAGER_H_
#define SOURCE_OPT_DEF_USE_MANAGER_H_
#include <list>
#include <set>
#include <unordered_map>
#include <utility>
#include <vector>
#include "source/opt/instruction.h"
@ -51,15 +49,17 @@ inline bool operator<(const Use& lhs, const Use& rhs) {
return lhs.operand_index < rhs.operand_index;
}
// Definition and user pair.
//
// The first element of the pair is the definition.
// The second element of the pair is the user.
//
// Definition should never be null. User can be null, however, such an entry
// should be used only for searching (e.g. all users of a particular definition)
// and never stored in a container.
using UserEntry = std::pair<Instruction*, Instruction*>;
struct UserEntry {
Instruction* def;
Instruction* user;
};
inline bool operator==(const UserEntry& lhs, const UserEntry& rhs) {
return lhs.def == rhs.def && lhs.user == rhs.user;
}
// Orders UserEntry for use in associative containers (i.e. less than ordering).
//
@ -72,24 +72,24 @@ using UserEntry = std::pair<Instruction*, Instruction*>;
// definition (i.e. using {def, nullptr}).
struct UserEntryLess {
bool operator()(const UserEntry& lhs, const UserEntry& rhs) const {
// If lhs.first and rhs.first are both null, fall through to checking the
// If lhs.def and rhs.def are both null, fall through to checking the
// second entries.
if (!lhs.first && rhs.first) return true;
if (lhs.first && !rhs.first) return false;
if (!lhs.def && rhs.def) return true;
if (lhs.def && !rhs.def) return false;
// If neither definition is null, then compare unique ids.
if (lhs.first && rhs.first) {
if (lhs.first->unique_id() < rhs.first->unique_id()) return true;
if (rhs.first->unique_id() < lhs.first->unique_id()) return false;
if (lhs.def && rhs.def) {
if (lhs.def->unique_id() < rhs.def->unique_id()) return true;
if (rhs.def->unique_id() < lhs.def->unique_id()) return false;
}
// Return false on equality.
if (!lhs.second && !rhs.second) return false;
if (!lhs.second) return true;
if (!rhs.second) return false;
if (!lhs.user && !rhs.user) return false;
if (!lhs.user) return true;
if (!rhs.user) return false;
// If neither user is null then compare unique ids.
return lhs.second->unique_id() < rhs.second->unique_id();
return lhs.user->unique_id() < rhs.user->unique_id();
}
};