[BOLT] Refactor handling of interproc refs

Summary:
Move handling of interprocedural references to BinaryContext.

Post-process indirect branches immediately after the CFG is built.

This is almost NFC. Since indirect branches are now post-processed
before the profile data is processed it interferes with the way the
profile data in YAML format is handled.

(cherry picked from FBD15456003)
This commit is contained in:
Maksim Panchenko
2019-05-22 11:26:58 -07:00
parent d047df12c5
commit be344c8de7
4 changed files with 98 additions and 91 deletions

View File

@@ -516,6 +516,84 @@ void BinaryContext::generateSymbolHashes() {
}
}
void BinaryContext::processInterproceduralReferences() {
for (auto &Pair : InterproceduralReferences) {
auto *FromBF = Pair.first;
auto Addr = Pair.second;
auto *ContainingFunction = getBinaryFunctionContainingAddress(Addr);
if (FromBF == ContainingFunction)
continue;
if (ContainingFunction) {
// Only a parent function (or a sibling) can reach its fragment.
if (ContainingFunction->IsFragment) {
assert(!FromBF->IsFragment &&
"only one cold fragment is supported at this time");
ContainingFunction->setParentFunction(FromBF);
FromBF->addFragment(ContainingFunction);
if (!HasRelocations) {
ContainingFunction->setSimple(false);
FromBF->setSimple(false);
}
if (opts::Verbosity >= 1) {
outs() << "BOLT-INFO: marking " << *ContainingFunction
<< " as a fragment of " << *FromBF << '\n';
}
continue;
}
if (ContainingFunction->getAddress() != Addr) {
ContainingFunction->addEntryPoint(Addr);
if (!HasRelocations) {
if (opts::Verbosity >= 1) {
errs() << "BOLT-WARNING: Function " << *ContainingFunction
<< " has internal BBs that are target of a reference "
<< "located in another function. Skipping the function.\n";
}
ContainingFunction->setSimple(false);
}
}
} else if (Addr) {
// Check if address falls in function padding space - this could be
// unmarked data in code. In this case adjust the padding space size.
auto Section = getSectionForAddress(Addr);
assert(Section && "cannot get section for referenced address");
if (!Section->isText())
continue;
// PLT requires special handling and could be ignored in this context.
StringRef SectionName = Section->getName();
if (SectionName == ".plt" || SectionName == ".plt.got")
continue;
if (HasRelocations) {
errs() << "BOLT-ERROR: cannot process binaries with unmarked "
<< "object in code at address 0x"
<< Twine::utohexstr(Addr) << " belonging to section "
<< SectionName << " in relocation mode.\n";
exit(1);
}
ContainingFunction =
getBinaryFunctionContainingAddress(Addr,
/*CheckPastEnd=*/false,
/*UseMaxSize=*/true);
// We are not going to overwrite non-simple functions, but for simple
// ones - adjust the padding size.
if (ContainingFunction && ContainingFunction->isSimple()) {
errs() << "BOLT-WARNING: function " << *ContainingFunction
<< " has an object detected in a padding region at address 0x"
<< Twine::utohexstr(Addr) << '\n';
ContainingFunction->setMaxSize(Addr -
ContainingFunction->getAddress());
}
}
}
InterproceduralReferences.clear();
}
void BinaryContext::postProcessSymbolTable() {
fixBinaryDataHoles();
bool Valid = true;

View File

@@ -594,6 +594,9 @@ public:
/// @}
/// Resolve inter-procedural dependencies.
void processInterproceduralReferences();
/// Perform any necessary post processing on the symbol table after
/// function disassembly is complete. This processing fixes top
/// level data holes and makes sure the symbol table is valid.

View File

@@ -1911,6 +1911,17 @@ bool BinaryFunction::buildCFG() {
// Update the state.
CurrentState = State::CFG;
// Make any necessary adjustments for indirect branches.
if (!postProcessIndirectBranches()) {
if (opts::Verbosity) {
errs() << "BOLT-WARNING: failed to post-process indirect branches for "
<< *this << '\n';
}
// In relocation mode we want to keep processing the function but avoid
// optimizing it.
setSimple(false);
}
return true;
}
@@ -1920,25 +1931,15 @@ void BinaryFunction::postProcessCFG() {
// to a tail call.
removeConditionalTailCalls();
// Make any necessary adjustments for indirect branches.
if (!postProcessIndirectBranches()) {
if (opts::Verbosity) {
errs() << "BOLT-WARNING: failed to post-process indirect branches for "
<< *this << '\n';
}
// In relocation mode we want to keep processing the function but avoid
// optimizing it.
setSimple(false);
} else {
postProcessProfile();
postProcessProfile();
// Eliminate inconsistencies between branch instructions and CFG.
postProcessBranches();
}
// Eliminate inconsistencies between branch instructions and CFG.
postProcessBranches();
calculateMacroOpFusionStats();
}
calculateMacroOpFusionStats();
// The final cleanup of intermediate structures.
clearList(IgnoredBranches);
clearList(EntryOffsets);

View File

@@ -2603,82 +2603,7 @@ void RewriteInstance::disassembleFunctions() {
if (opts::PrintAll || opts::PrintDisasm)
Function.print(outs(), "after disassembly", true);
// Post-process inter-procedural references ASAP as it may affect
// functions we are about to disassemble next.
for (auto &Pair : BC->InterproceduralReferences) {
auto *FromBF = Pair.first;
auto Addr = Pair.second;
auto *ContainingFunction = BC->getBinaryFunctionContainingAddress(Addr);
if (FromBF == ContainingFunction)
continue;
if (ContainingFunction) {
// Only a parent function (or a sibling) can reach its fragment.
if (ContainingFunction->IsFragment) {
assert(!FromBF->IsFragment &&
"only one cold fragment is supported at this time");
ContainingFunction->setParentFunction(FromBF);
FromBF->addFragment(ContainingFunction);
if (!BC->HasRelocations) {
ContainingFunction->setSimple(false);
FromBF->setSimple(false);
}
if (opts::Verbosity >= 1) {
outs() << "BOLT-INFO: marking " << *ContainingFunction
<< " as a fragment of " << *FromBF << '\n';
}
continue;
}
if (ContainingFunction->getAddress() != Addr) {
ContainingFunction->addEntryPoint(Addr);
if (!BC->HasRelocations) {
if (opts::Verbosity >= 1) {
errs() << "BOLT-WARNING: Function " << *ContainingFunction
<< " has internal BBs that are target of a reference "
<< "located in another function. Skipping the function.\n";
}
ContainingFunction->setSimple(false);
}
}
} else if (Addr) {
// Check if address falls in function padding space - this could be
// unmarked data in code. In this case adjust the padding space size.
auto Section = BC->getSectionForAddress(Addr);
assert(Section && "cannot get section for referenced address");
if (!Section->isText())
continue;
// PLT requires special handling and could be ignored in this context.
StringRef SectionName = Section->getName();
if (SectionName == ".plt" || SectionName == ".plt.got")
continue;
if (BC->HasRelocations) {
errs() << "BOLT-ERROR: cannot process binaries with unmarked "
<< "object in code at address 0x"
<< Twine::utohexstr(Addr) << " belonging to section "
<< SectionName << " in relocation mode.\n";
exit(1);
}
ContainingFunction =
BC->getBinaryFunctionContainingAddress(Addr,
/*CheckPastEnd=*/false,
/*UseMaxSize=*/true);
// We are not going to overwrite non-simple functions, but for simple
// ones - adjust the padding size.
if (ContainingFunction && ContainingFunction->isSimple()) {
errs() << "BOLT-WARNING: function " << *ContainingFunction
<< " has an object detected in a padding region at address 0x"
<< Twine::utohexstr(Addr) << '\n';
ContainingFunction->setMaxSize(Addr -
ContainingFunction->getAddress());
}
}
}
BC->InterproceduralReferences.clear();
BC->processInterproceduralReferences();
}
for (auto &BFI : BC->getBinaryFunctions()) {