Skip to content

Commit

Permalink
[BOLT][NFC] Move ICF pass into a separate file
Browse files Browse the repository at this point in the history
Summary:
Consolidate code used by identical code folding under
Passes/IdenticalCodeFolding.cpp.

(cherry picked from FBD8109916)
  • Loading branch information
maksfb committed May 22, 2018
1 parent 6302e18 commit 929b090
Show file tree
Hide file tree
Showing 10 changed files with 495 additions and 436 deletions.
4 changes: 2 additions & 2 deletions bolt/src/BinaryBasicBlock.h
Expand Up @@ -825,9 +825,9 @@ class BinaryBasicBlock {
LayoutIndex = Index;
}

/// FIXME
/// Needed by graph traits.
BinaryFunction *getParent() const {
return nullptr;
return getFunction();
}

private:
Expand Down
170 changes: 0 additions & 170 deletions bolt/src/BinaryFunction.cpp
Expand Up @@ -2981,136 +2981,6 @@ BinaryFunction::BasicBlockOrderType BinaryFunction::dfs() const {
return DFS;
}

bool BinaryFunction::isIdenticalWith(const BinaryFunction &OtherBF,
bool IgnoreSymbols,
bool UseDFS) const {
assert(hasCFG() && OtherBF.hasCFG() && "both functions should have CFG");

// Compare the two functions, one basic block at a time.
// Currently we require two identical basic blocks to have identical
// instruction sequences and the same index in their corresponding
// functions. The latter is important for CFG equality.

if (layout_size() != OtherBF.layout_size())
return false;

// Comparing multi-entry functions could be non-trivial.
if (isMultiEntry() || OtherBF.isMultiEntry())
return false;

// Process both functions in either DFS or existing order.
const auto &Order = UseDFS ? dfs() : BasicBlocksLayout;
const auto &OtherOrder = UseDFS ? OtherBF.dfs() : OtherBF.BasicBlocksLayout;

auto BBI = OtherOrder.begin();
for (const auto *BB : Order) {
const auto *OtherBB = *BBI;

if (BB->getLayoutIndex() != OtherBB->getLayoutIndex())
return false;

// Compare successor basic blocks.
// NOTE: the comparison for jump tables is only partially verified here.
if (BB->succ_size() != OtherBB->succ_size())
return false;

auto SuccBBI = OtherBB->succ_begin();
for (const auto *SuccBB : BB->successors()) {
const auto *SuccOtherBB = *SuccBBI;
if (SuccBB->getLayoutIndex() != SuccOtherBB->getLayoutIndex())
return false;
++SuccBBI;
}

// Compare all instructions including pseudos.
auto I = BB->begin(), E = BB->end();
auto OtherI = OtherBB->begin(), OtherE = OtherBB->end();
while (I != E && OtherI != OtherE) {

bool Identical;
if (IgnoreSymbols) {
Identical =
isInstrEquivalentWith(*I, *BB, *OtherI, *OtherBB, OtherBF,
[](const MCSymbol *A, const MCSymbol *B) {
return true;
});
} else {
// Compare symbols.
auto AreSymbolsIdentical = [&] (const MCSymbol *A, const MCSymbol *B) {
if (A == B)
return true;

// All local symbols are considered identical since they affect a
// control flow and we check the control flow separately.
// If a local symbol is escaped, then the function (potentially) has
// multiple entry points and we exclude such functions from
// comparison.
if (A->isTemporary() && B->isTemporary())
return true;

// Compare symbols as functions.
const auto *FunctionA = BC.getFunctionForSymbol(A);
const auto *FunctionB = BC.getFunctionForSymbol(B);
if (FunctionA && FunctionB) {
// Self-referencing functions and recursive calls.
if (FunctionA == this && FunctionB == &OtherBF)
return true;
return FunctionA == FunctionB;
}

// Check if symbols are jump tables.
auto *SIA = BC.getBinaryDataByName(A->getName());
if (!SIA)
return false;
auto *SIB = BC.getBinaryDataByName(B->getName());
if (!SIB)
return false;

assert((SIA->getAddress() != SIB->getAddress()) &&
"different symbols should not have the same value");

const auto *JumpTableA =
getJumpTableContainingAddress(SIA->getAddress());
if (!JumpTableA)
return false;

const auto *JumpTableB =
OtherBF.getJumpTableContainingAddress(SIB->getAddress());
if (!JumpTableB)
return false;

if ((SIA->getAddress() - JumpTableA->getAddress()) !=
(SIB->getAddress() - JumpTableB->getAddress()))
return false;

return equalJumpTables(JumpTableA, JumpTableB, OtherBF);
};

Identical =
isInstrEquivalentWith(*I, *BB, *OtherI, *OtherBB, OtherBF,
AreSymbolsIdentical);
}

if (!Identical)
return false;

++I; ++OtherI;
}

// One of the identical blocks may have a trailing unconditional jump that
// is ignored for CFG purposes.
auto *TrailingInstr = (I != E ? &(*I)
: (OtherI != OtherE ? &(*OtherI) : 0));
if (TrailingInstr && !BC.MIB->isUnconditionalBranch(*TrailingInstr)) {
return false;
}

++BBI;
}

return true;
}

std::string BinaryFunction::generateJumpTableName(uint64_t Address) const {
auto *JT = getJumpTableContainingAddress(Address);
size_t Id;
Expand All @@ -3129,46 +2999,6 @@ std::string BinaryFunction::generateJumpTableName(uint64_t Address) const {
(Offset ? ("." + std::to_string(Offset)) : ""));
}

bool BinaryFunction::equalJumpTables(const JumpTable *JumpTableA,
const JumpTable *JumpTableB,
const BinaryFunction &BFB) const {
if (JumpTableA->EntrySize != JumpTableB->EntrySize)
return false;

if (JumpTableA->Type != JumpTableB->Type)
return false;

if (JumpTableA->getSize() != JumpTableB->getSize())
return false;

for (uint64_t Index = 0; Index < JumpTableA->Entries.size(); ++Index) {
const auto *LabelA = JumpTableA->Entries[Index];
const auto *LabelB = JumpTableB->Entries[Index];

const auto *TargetA = getBasicBlockForLabel(LabelA);
const auto *TargetB = BFB.getBasicBlockForLabel(LabelB);

if (!TargetA || !TargetB) {
assert((TargetA || LabelA == getFunctionEndLabel()) &&
"no target basic block found");
assert((TargetB || LabelB == BFB.getFunctionEndLabel()) &&
"no target basic block found");

if (TargetA != TargetB)
return false;

continue;
}

assert(TargetA && TargetB && "cannot locate target block(s)");

if (TargetA->getLayoutIndex() != TargetB->getLayoutIndex())
return false;
}

return true;
}

std::size_t BinaryFunction::hash(bool Recompute, bool UseDFS) const {
if (size() == 0)
return 0;
Expand Down
123 changes: 24 additions & 99 deletions bolt/src/BinaryFunction.h
Expand Up @@ -424,60 +424,6 @@ class BinaryFunction {
/// Synchronize branch instructions with CFG.
void postProcessBranches();

/// Helper function that compares an instruction of this function to the
/// given instruction of the given function. The functions should have
/// identical CFG.
template <class Compare>
bool isInstrEquivalentWith(
const MCInst &InstA, const BinaryBasicBlock &BBA,
const MCInst &InstB, const BinaryBasicBlock &BBB,
const BinaryFunction &BFB, Compare Comp) const {
if (InstA.getOpcode() != InstB.getOpcode()) {
return false;
}

// In this function we check for special conditions:
//
// * instructions with landing pads
//
// Most of the common cases should be handled by MCPlus::equals()
// that compares regular instruction operands.
//
// NB: there's no need to compare jump table indirect jump instructions
// separately as jump tables are handled by comparing corresponding
// symbols.
const auto EHInfoA = BC.MIB->getEHInfo(InstA);
const auto EHInfoB = BC.MIB->getEHInfo(InstB);

if (EHInfoA || EHInfoB) {
if (!EHInfoA && (EHInfoB->first || EHInfoB->second))
return false;

if (!EHInfoB && (EHInfoA->first || EHInfoA->second))
return false;

if (EHInfoA && EHInfoB) {
// Action indices should match.
if (EHInfoA->second != EHInfoB->second)
return false;

if (!EHInfoA->first != !EHInfoB->first)
return false;

if (EHInfoA->first && EHInfoB->first) {
const auto *LPA = BBA.getLandingPad(EHInfoA->first);
const auto *LPB = BBB.getLandingPad(EHInfoB->first);
assert(LPA && LPB && "cannot locate landing pad(s)");

if (LPA->getLayoutIndex() != LPB->getLayoutIndex())
return false;
}
}
}

return BC.MIB->equals(InstA, InstB, Comp);
}

/// Recompute landing pad information for the function and all its blocks.
void recomputeLandingPads();

Expand Down Expand Up @@ -583,45 +529,16 @@ class BinaryFunction {
/// jump table names.
mutable std::map<uint64_t, size_t> JumpTableIds;

/// Generate a unique name for this jump table at the given address that should
/// be repeatable no matter what the start address of the table is.
/// Generate a unique name for this jump table at the given address that
/// should be repeatable no matter what the start address of the table is.
std::string generateJumpTableName(uint64_t Address) const;

/// Return jump table that covers a given \p Address in memory.
JumpTable *getJumpTableContainingAddress(uint64_t Address) {
auto JTI = JumpTables.upper_bound(Address);
if (JTI == JumpTables.begin())
return nullptr;
--JTI;
if (JTI->first + JTI->second->getSize() > Address) {
return JTI->second;
}
return nullptr;
}

const JumpTable *getJumpTableContainingAddress(uint64_t Address) const {
auto JTI = JumpTables.upper_bound(Address);
if (JTI == JumpTables.begin())
return nullptr;
--JTI;
if (JTI->first + JTI->second->getSize() > Address) {
return JTI->second;
}
return nullptr;
}

/// Iterate over all jump tables associated with this function.
iterator_range<std::map<uint64_t, JumpTable *>::const_iterator>
jumpTables() const {
return make_range(JumpTables.begin(), JumpTables.end());
}

/// Compare two jump tables in 2 functions. The function relies on consistent
/// ordering of basic blocks in both binary functions (e.g. DFS).
bool equalJumpTables(const JumpTable *JumpTableA,
const JumpTable *JumpTableB,
const BinaryFunction &BFB) const;

/// All jump table sites in the function.
std::vector<std::pair<uint64_t, uint64_t>> JTSites;

Expand Down Expand Up @@ -767,7 +684,6 @@ class BinaryFunction {
}

public:

BinaryFunction(BinaryFunction &&) = default;

using iterator = pointee_iterator<BasicBlockListType::iterator>;
Expand Down Expand Up @@ -872,6 +788,11 @@ class BinaryFunction {
}
}

/// Return current basic block layout.
const BasicBlockOrderType &getLayout() const {
return BasicBlocksLayout;
}

/// Return a list of basic blocks sorted using DFS and update layout indices
/// using the same order. Does not modify the current layout.
BasicBlockOrderType dfs() const;
Expand Down Expand Up @@ -985,6 +906,23 @@ class BinaryFunction {
/// CFG is constructed or while instruction offsets are available in CFG.
MCInst *getInstructionAtOffset(uint64_t Offset);

/// Return jump table that covers a given \p Address in memory.
JumpTable *getJumpTableContainingAddress(uint64_t Address) {
auto JTI = JumpTables.upper_bound(Address);
if (JTI == JumpTables.begin())
return nullptr;
--JTI;
if (JTI->first + JTI->second->getSize() > Address) {
return JTI->second;
}
return nullptr;
}

const JumpTable *getJumpTableContainingAddress(uint64_t Address) const {
return const_cast<BinaryFunction *>(this)->
getJumpTableContainingAddress(Address);
}

/// Return the name of the function as extracted from the binary file.
/// If the function has multiple names - return the last one
/// followed by "(*#<numnames>)".
Expand Down Expand Up @@ -2125,19 +2063,6 @@ class BinaryFunction {
/// Convert function-level branch data into instruction annotations.
void convertBranchData();

/// Returns true if this function has identical code and CFG with
/// the given function \p BF.
///
/// If \p IgnoreSymbols is set to true, then symbolic operands are ignored
/// during comparison.
///
/// If \p UseDFS is set to true, then compute DFS of each function and use
/// is for CFG equivalency. Potentially it will help to catch more cases,
/// but is slower.
bool isIdenticalWith(const BinaryFunction &BF,
bool IgnoreSymbols = false,
bool UseDFS = false) const;

/// Returns a hash value for the function. To be used for ICF. Two congruent
/// functions (functions with different symbolic references but identical
/// otherwise) are required to have identical hashes.
Expand Down
1 change: 1 addition & 0 deletions bolt/src/BinaryPassManager.cpp
Expand Up @@ -13,6 +13,7 @@
#include "Passes/Aligner.h"
#include "Passes/AllocCombiner.h"
#include "Passes/FrameOptimizer.h"
#include "Passes/IdenticalCodeFolding.h"
#include "Passes/IndirectCallPromotion.h"
#include "Passes/Inliner.h"
#include "Passes/LongJmp.h"
Expand Down
2 changes: 1 addition & 1 deletion bolt/src/BoltDiff.cpp
Expand Up @@ -13,7 +13,7 @@
//===----------------------------------------------------------------------===//

#include "RewriteInstance.h"
#include "Passes/BinaryPasses.h"
#include "Passes/IdenticalCodeFolding.h"
#include "llvm/Support/CommandLine.h"

#undef DEBUG_TYPE
Expand Down

0 comments on commit 929b090

Please sign in to comment.