-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[BOLT] Refactor MCInstReference and move it to Core (NFC) #155846
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.
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
Changes from 4 commits
4a0d3e8
eb32eb8
3d8ac8c
2a25f3a
b068098
0558f1b
5a7414c
ec6bff9
2fc734c
66e0fb4
9590ffc
5923bab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,175 @@ | ||
//===- bolt/Core/MCInstUtils.h ----------------------------------*- C++ -*-===// | ||
// | ||
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
// See https://llvm.org/LICENSE.txt for license information. | ||
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
#ifndef BOLT_CORE_MCINSTUTILS_H | ||
#define BOLT_CORE_MCINSTUTILS_H | ||
|
||
#include "bolt/Core/BinaryBasicBlock.h" | ||
#include <map> | ||
#include <variant> | ||
|
||
namespace llvm { | ||
class MCCodeEmitter; | ||
} | ||
|
||
namespace llvm { | ||
namespace bolt { | ||
|
||
class BinaryFunction; | ||
|
||
/// MCInstReference represents a reference to a constant MCInst as stored either | ||
/// in a BinaryFunction (i.e. before a CFG is created), or in a BinaryBasicBlock | ||
/// (after a CFG is created). | ||
/// | ||
/// The reference may be invalidated when the function containing the referenced | ||
/// instruction is modified. | ||
class MCInstReference { | ||
public: | ||
using nocfg_const_iterator = std::map<uint32_t, MCInst>::const_iterator; | ||
|
||
/// Constructs an empty reference. | ||
MCInstReference() : Reference(RefInBB(nullptr, /*Index=*/0)) {} | ||
/// Constructs a reference to the instruction inside the basic block. | ||
MCInstReference(const BinaryBasicBlock *BB, const MCInst *Inst) | ||
: Reference(RefInBB(BB, getInstIndexInBB(BB, Inst))) {} | ||
/// Constructs a reference to the instruction inside the basic block. | ||
MCInstReference(const BinaryBasicBlock *BB, unsigned Index) | ||
: Reference(RefInBB(BB, Index)) { | ||
assert(BB && "Basic block should not be nullptr"); | ||
} | ||
/// Constructs a reference to the instruction inside the function without | ||
/// CFG information. | ||
MCInstReference(const BinaryFunction *BF, nocfg_const_iterator It) | ||
: Reference(RefInBF(BF, It)) { | ||
assert(BF && "Function should not be nullptr"); | ||
} | ||
|
||
/// Locates an instruction inside a function and returns a reference. | ||
static MCInstReference get(const MCInst *Inst, const BinaryFunction &BF); | ||
|
||
bool operator==(const MCInstReference &Other) const { | ||
return Reference == Other.Reference; | ||
} | ||
|
||
const MCInst &getMCInst() const { | ||
assert(!empty() && "Empty reference"); | ||
if (auto *Ref = tryGetRefInBB()) { | ||
[[maybe_unused]] unsigned NumInstructions = Ref->BB->size(); | ||
assert(Ref->Index < NumInstructions && "Invalid reference"); | ||
return Ref->BB->getInstructionAtIndex(Ref->Index); | ||
} | ||
return getRefInBF().It->second; | ||
} | ||
|
||
operator const MCInst &() const { return getMCInst(); } | ||
|
||
bool empty() const { | ||
if (auto *Ref = tryGetRefInBB()) | ||
return Ref->BB == nullptr; | ||
return getRefInBF().BF == nullptr; | ||
} | ||
|
||
bool hasCFG() const { return !empty() && tryGetRefInBB() != nullptr; } | ||
|
||
const BinaryFunction *getFunction() const { | ||
assert(!empty() && "Empty reference"); | ||
if (auto *Ref = tryGetRefInBB()) | ||
return Ref->BB->getFunction(); | ||
return getRefInBF().BF; | ||
} | ||
|
||
const BinaryBasicBlock *getBasicBlock() const { | ||
assert(!empty() && "Empty reference"); | ||
if (auto *Ref = tryGetRefInBB()) | ||
return Ref->BB; | ||
return nullptr; | ||
} | ||
|
||
// MCCodeEmitter is not thread safe. | ||
uint64_t getAddress(const MCCodeEmitter *Emitter = nullptr) const; | ||
|
||
|
||
raw_ostream &print(raw_ostream &OS) const; | ||
|
||
private: | ||
static unsigned getInstIndexInBB(const BinaryBasicBlock *BB, | ||
const MCInst *Inst) { | ||
assert(BB && Inst && "Neither BB nor Inst should be nullptr"); | ||
// Usage of pointer arithmetic assumes the instructions are stored in a | ||
// vector, see BasicBlockStorageIsVector in MCInstUtils.cpp. | ||
const MCInst *FirstInstInBB = &*BB->begin(); | ||
return Inst - FirstInstInBB; | ||
} | ||
|
||
// Two cases are possible: | ||
// * functions with CFG reconstructed - a function stores a collection of | ||
// basic blocks, each basic block stores a contiguous vector of MCInst | ||
// * functions without CFG - there are no basic blocks created, | ||
// the instructions are directly stored in std::map in BinaryFunction | ||
// | ||
// In both cases, the direct parent of MCInst is stored together with an | ||
// index or iterator pointing to the instruction. | ||
|
||
// Helper struct: CFG is available, the direct parent is a basic block. | ||
struct RefInBB { | ||
RefInBB(const BinaryBasicBlock *BB, unsigned Index) | ||
: BB(BB), Index(Index) {} | ||
RefInBB(const RefInBB &Other) = default; | ||
RefInBB &operator=(const RefInBB &Other) = default; | ||
|
||
const BinaryBasicBlock *BB; | ||
unsigned Index; | ||
|
||
bool operator==(const RefInBB &Other) const { | ||
return BB == Other.BB && Index == Other.Index; | ||
} | ||
}; | ||
|
||
// Helper struct: CFG is *not* available, the direct parent is a function, | ||
// iterator's type is std::map<uint32_t, MCInst>::iterator (the mapped value | ||
// is an instruction's offset). | ||
struct RefInBF { | ||
RefInBF(const BinaryFunction *BF, nocfg_const_iterator It) | ||
: BF(BF), It(It) {} | ||
RefInBF(const RefInBF &Other) = default; | ||
RefInBF &operator=(const RefInBF &Other) = default; | ||
|
||
const BinaryFunction *BF; | ||
nocfg_const_iterator It; | ||
|
||
bool operator==(const RefInBF &Other) const { | ||
return BF == Other.BF && It->first == Other.It->first; | ||
} | ||
}; | ||
|
||
std::variant<RefInBB, RefInBF> Reference; | ||
|
||
// Utility methods to be used like this: | ||
// | ||
// if (auto *Ref = tryGetRefInBB()) | ||
// return Ref->doSomething(...); | ||
// return getRefInBF().doSomethingElse(...); | ||
const RefInBB *tryGetRefInBB() const { | ||
assert(std::get_if<RefInBB>(&Reference) || | ||
std::get_if<RefInBF>(&Reference)); | ||
return std::get_if<RefInBB>(&Reference); | ||
} | ||
const RefInBF &getRefInBF() const { | ||
assert(std::get_if<RefInBF>(&Reference)); | ||
return *std::get_if<RefInBF>(&Reference); | ||
} | ||
}; | ||
|
||
static inline raw_ostream &operator<<(raw_ostream &OS, | ||
const MCInstReference &Ref) { | ||
return Ref.print(OS); | ||
} | ||
|
||
} // namespace bolt | ||
} // namespace llvm | ||
|
||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, if you don't expect to pass/handle
nullptr
as a pointer, prefer a reference. You might need a special case for an empty reference.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for pointing this out, I replaced a number of pointer-typed arguments with references in 0558f1b and now it should be clearer to the user when empty references can be returned (the privately-used
RefInBB
andRefInBF
helper classes still use pointers, though). Furthermore, I madeMCInstReference::get
accept both arguments by reference in 5a7414c: while it could have sense to return empty reference for(nullptr, SomeFunction)
arguments (but even this usage is slightly questionable), it is definitely strange to pass unrelated instruction and function and expect theMCInstReference::get
to silently return an empty reference.