Skip to content

Commit

Permalink
[DomTree] Cleanup Update and LegalizeUpdate API moved to Support header.
Browse files Browse the repository at this point in the history
Summary:
Clean-up following D50479.
Make Update and LegalizeUpdate refer to the utilities in Support/CFGUpdate.

Reviewers: kuhar

Subscribers: sanjoy, jlebar, mgrang, llvm-commits

Differential Revision: https://reviews.llvm.org/D50669

llvm-svn: 339694
  • Loading branch information
alinas committed Aug 14, 2018
1 parent 2c0bc8b commit 148c445
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 117 deletions.
6 changes: 3 additions & 3 deletions llvm/include/llvm/IR/Dominators.h
Expand Up @@ -37,13 +37,13 @@ extern template class DomTreeNodeBase<BasicBlock>;
extern template class DominatorTreeBase<BasicBlock, false>; // DomTree
extern template class DominatorTreeBase<BasicBlock, true>; // PostDomTree

extern template class cfg::Update<BasicBlock *>;

namespace DomTreeBuilder {
using BBDomTree = DomTreeBase<BasicBlock>;
using BBPostDomTree = PostDomTreeBase<BasicBlock>;

extern template struct Update<BasicBlock *>;

using BBUpdates = ArrayRef<Update<BasicBlock *>>;
using BBUpdates = ArrayRef<llvm::cfg::Update<BasicBlock *>>;

extern template void Calculate<BBDomTree>(BBDomTree &DT);
extern template void Calculate<BBPostDomTree>(BBPostDomTree &DT);
Expand Down
49 changes: 10 additions & 39 deletions llvm/include/llvm/Support/GenericDomTree.h
Expand Up @@ -24,6 +24,14 @@
#ifndef LLVM_SUPPORT_GENERICDOMTREE_H
#define LLVM_SUPPORT_GENERICDOMTREE_H

#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/GraphTraits.h"
#include "llvm/ADT/PointerIntPair.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/CFGUpdate.h"
#include "llvm/Support/raw_ostream.h"
#include <algorithm>
#include <cassert>
#include <cstddef>
Expand All @@ -32,13 +40,6 @@
#include <type_traits>
#include <utility>
#include <vector>
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/GraphTraits.h"
#include "llvm/ADT/PointerIntPair.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/raw_ostream.h"

namespace llvm {

Expand Down Expand Up @@ -199,36 +200,6 @@ template <typename DomTreeT>
void DeleteEdge(DomTreeT &DT, typename DomTreeT::NodePtr From,
typename DomTreeT::NodePtr To);

// UpdateKind and Update are used by the batch update API and it's easiest to
// define them here.
enum class UpdateKind : unsigned char { Insert, Delete };

template <typename NodePtr>
struct Update {
using NodeKindPair = PointerIntPair<NodePtr, 1, UpdateKind>;

NodePtr From;
NodeKindPair ToAndKind;

Update(UpdateKind Kind, NodePtr From, NodePtr To)
: From(From), ToAndKind(To, Kind) {}

UpdateKind getKind() const { return ToAndKind.getInt(); }
NodePtr getFrom() const { return From; }
NodePtr getTo() const { return ToAndKind.getPointer(); }
bool operator==(const Update &RHS) const {
return From == RHS.From && ToAndKind == RHS.ToAndKind;
}

friend raw_ostream &operator<<(raw_ostream &OS, const Update &U) {
OS << (U.getKind() == UpdateKind::Insert ? "Insert " : "Delete ");
U.getFrom()->printAsOperand(OS, false);
OS << " -> ";
U.getTo()->printAsOperand(OS, false);
return OS;
}
};

template <typename DomTreeT>
void ApplyUpdates(DomTreeT &DT,
ArrayRef<typename DomTreeT::UpdateType> Updates);
Expand All @@ -254,8 +225,8 @@ class DominatorTreeBase {
using ParentType = typename std::remove_pointer<ParentPtr>::type;
static constexpr bool IsPostDominator = IsPostDom;

using UpdateType = DomTreeBuilder::Update<NodePtr>;
using UpdateKind = DomTreeBuilder::UpdateKind;
using UpdateType = cfg::Update<NodePtr>;
using UpdateKind = cfg::UpdateKind;
static constexpr UpdateKind Insert = UpdateKind::Insert;
static constexpr UpdateKind Delete = UpdateKind::Delete;

Expand Down
80 changes: 10 additions & 70 deletions llvm/include/llvm/Support/GenericDomTreeConstruction.h
Expand Up @@ -71,6 +71,7 @@ struct SemiNCAInfo {
DenseMap<NodePtr, InfoRec> NodeToInfo;

using UpdateT = typename DomTreeT::UpdateType;
using UpdateKind = typename DomTreeT::UpdateKind;
struct BatchUpdateInfo {
SmallVector<UpdateT, 4> Updates;
using NodePtrAndKind = PointerIntPair<NodePtr, 1, UpdateKind>;
Expand Down Expand Up @@ -1166,7 +1167,8 @@ struct SemiNCAInfo {
}

BatchUpdateInfo BUI;
LegalizeUpdates(Updates, BUI.Updates);
LLVM_DEBUG(dbgs() << "Legalizing " << BUI.Updates.size() << " updates\n");
cfg::LegalizeUpdates<NodePtr>(Updates, BUI.Updates, IsPostDom);

const size_t NumLegalized = BUI.Updates.size();
BUI.FutureSuccessors.reserve(NumLegalized);
Expand All @@ -1182,8 +1184,11 @@ struct SemiNCAInfo {

LLVM_DEBUG(dbgs() << "About to apply " << NumLegalized << " updates\n");
LLVM_DEBUG(if (NumLegalized < 32) for (const auto &U
: reverse(BUI.Updates)) dbgs()
<< '\t' << U << "\n");
: reverse(BUI.Updates)) {
dbgs() << "\t";
U.dump();
dbgs() << "\n";
});
LLVM_DEBUG(dbgs() << "\n");

// If the DominatorTree was recalculated at some point, stop the batch
Expand All @@ -1193,76 +1198,11 @@ struct SemiNCAInfo {
ApplyNextUpdate(DT, BUI);
}

// This function serves double purpose:
// a) It removes redundant updates, which makes it easier to reverse-apply
// them when traversing CFG.
// b) It optimizes away updates that cancel each other out, as the end result
// is the same.
//
// It relies on the property of the incremental updates that says that the
// order of updates doesn't matter. This allows us to reorder them and end up
// with the exact same DomTree every time.
//
// Following the same logic, the function doesn't care about the order of
// input updates, so it's OK to pass it an unordered sequence of updates, that
// doesn't make sense when applied sequentially, eg. performing double
// insertions or deletions and then doing an opposite update.
//
// In the future, it should be possible to schedule updates in way that
// minimizes the amount of work needed done during incremental updates.
static void LegalizeUpdates(ArrayRef<UpdateT> AllUpdates,
SmallVectorImpl<UpdateT> &Result) {
LLVM_DEBUG(dbgs() << "Legalizing " << AllUpdates.size() << " updates\n");
// Count the total number of inserions of each edge.
// Each insertion adds 1 and deletion subtracts 1. The end number should be
// one of {-1 (deletion), 0 (NOP), +1 (insertion)}. Otherwise, the sequence
// of updates contains multiple updates of the same kind and we assert for
// that case.
SmallDenseMap<std::pair<NodePtr, NodePtr>, int, 4> Operations;
Operations.reserve(AllUpdates.size());

for (const auto &U : AllUpdates) {
NodePtr From = U.getFrom();
NodePtr To = U.getTo();
if (IsPostDom) std::swap(From, To); // Reverse edge for postdominators.

Operations[{From, To}] += (U.getKind() == UpdateKind::Insert ? 1 : -1);
}

Result.clear();
Result.reserve(Operations.size());
for (auto &Op : Operations) {
const int NumInsertions = Op.second;
assert(std::abs(NumInsertions) <= 1 && "Unbalanced operations!");
if (NumInsertions == 0) continue;
const UpdateKind UK =
NumInsertions > 0 ? UpdateKind::Insert : UpdateKind::Delete;
Result.push_back({UK, Op.first.first, Op.first.second});
}

// Make the order consistent by not relying on pointer values within the
// set. Reuse the old Operations map.
// In the future, we should sort by something else to minimize the amount
// of work needed to perform the series of updates.
for (size_t i = 0, e = AllUpdates.size(); i != e; ++i) {
const auto &U = AllUpdates[i];
if (!IsPostDom)
Operations[{U.getFrom(), U.getTo()}] = int(i);
else
Operations[{U.getTo(), U.getFrom()}] = int(i);
}

llvm::sort(Result.begin(), Result.end(),
[&Operations](const UpdateT &A, const UpdateT &B) {
return Operations[{A.getFrom(), A.getTo()}] >
Operations[{B.getFrom(), B.getTo()}];
});
}

static void ApplyNextUpdate(DomTreeT &DT, BatchUpdateInfo &BUI) {
assert(!BUI.Updates.empty() && "No updates to apply!");
UpdateT CurrentUpdate = BUI.Updates.pop_back_val();
LLVM_DEBUG(dbgs() << "Applying update: " << CurrentUpdate << "\n");
LLVM_DEBUG(dbgs() << "Applying update: ");
LLVM_DEBUG(CurrentUpdate.dump(); dbgs() << "\n");

// Move to the next snapshot of the CFG by removing the reverse-applied
// current update. Since updates are performed in the same order they are
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/IR/Dominators.cpp
Expand Up @@ -67,7 +67,7 @@ template class llvm::DomTreeNodeBase<BasicBlock>;
template class llvm::DominatorTreeBase<BasicBlock, false>; // DomTreeBase
template class llvm::DominatorTreeBase<BasicBlock, true>; // PostDomTreeBase

template struct llvm::DomTreeBuilder::Update<BasicBlock *>;
template class llvm::cfg::Update<BasicBlock *>;

template void llvm::DomTreeBuilder::Calculate<DomTreeBuilder::BBDomTree>(
DomTreeBuilder::BBDomTree &DT);
Expand Down
8 changes: 4 additions & 4 deletions llvm/unittests/IR/DominatorTreeBatchUpdatesTest.cpp
Expand Up @@ -58,9 +58,9 @@ TEST(DominatorTreeBatchUpdates, LegalizeDomUpdates) {
{Insert, B, C}, {Insert, C, D}, {Delete, B, C}, {Insert, B, C},
{Insert, B, D}, {Delete, C, D}, {Delete, A, B}};
SmallVector<DomUpdate, 4> Legalized;
DomSNCA::LegalizeUpdates(Updates, Legalized);
cfg::LegalizeUpdates<BasicBlock *>(Updates, Legalized, false);
LLVM_DEBUG(dbgs() << "Legalized updates:\t");
LLVM_DEBUG(for (auto &U : Legalized) dbgs() << U << ", ");
LLVM_DEBUG(for (auto &U : Legalized) { U.dump(); dbgs() << ", "; });
LLVM_DEBUG(dbgs() << "\n");
EXPECT_EQ(Legalized.size(), 3UL);
EXPECT_NE(llvm::find(Legalized, DomUpdate{Insert, B, C}), Legalized.end());
Expand All @@ -81,9 +81,9 @@ TEST(DominatorTreeBatchUpdates, LegalizePostDomUpdates) {
{Insert, B, C}, {Insert, C, D}, {Delete, B, C}, {Insert, B, C},
{Insert, B, D}, {Delete, C, D}, {Delete, A, B}};
SmallVector<DomUpdate, 4> Legalized;
PostDomSNCA::LegalizeUpdates(Updates, Legalized);
cfg::LegalizeUpdates<BasicBlock *>(Updates, Legalized, true);
LLVM_DEBUG(dbgs() << "Legalized postdom updates:\t");
LLVM_DEBUG(for (auto &U : Legalized) dbgs() << U << ", ");
LLVM_DEBUG(for (auto &U : Legalized) { U.dump(); dbgs() << ", "; });
LLVM_DEBUG(dbgs() << "\n");
EXPECT_EQ(Legalized.size(), 3UL);
EXPECT_NE(llvm::find(Legalized, DomUpdate{Insert, C, B}), Legalized.end());
Expand Down

0 comments on commit 148c445

Please sign in to comment.