Subzero: Clean up a few areas.
1. Use a reverse_range() adaptor for range-based reverse iteration through containers.
2. Remove the emitting of the commented llvm-mc command line.
3. Remove a few TODOs.
4. For commented-out declarations within a class T like this:
// T(const T&) = delete;
// T &operator=(const T &) = delete;
Replace them with this:
T(const T&) = default;
T &operator=(const T &) = default;
And try to keep them private where possible.
5. Make LivenessNode and TimerTreeNode into internal classes.
BUG= none
R=jvoung@chromium.org
Review URL: https://codereview.chromium.org/830303003
diff --git a/src/IceCfg.cpp b/src/IceCfg.cpp
index 9726ff9..d3c47d4 100644
--- a/src/IceCfg.cpp
+++ b/src/IceCfg.cpp
@@ -269,8 +269,7 @@
llvm::BitVector NeedToProcess(Nodes.size(), true);
while (NeedToProcess.any()) {
// Iterate in reverse topological order to speed up convergence.
- for (auto I = Nodes.rbegin(), E = Nodes.rend(); I != E; ++I) {
- CfgNode *Node = *I;
+ for (CfgNode *Node : reverse_range(Nodes)) {
if (NeedToProcess[Node->getIndex()]) {
NeedToProcess[Node->getIndex()] = false;
bool Changed = Node->liveness(getLiveness());
@@ -442,16 +441,6 @@
dump("After recomputing liveness for -decorate-asm");
}
Ostream &Str = Ctx->getStrEmit();
- if (!Ctx->testAndSetHasEmittedFirstMethod()) {
- // Print a helpful command for assembling the output.
- // TODO: have the Target emit the header
- // TODO: need a per-file emit in addition to per-CFG
- Str << "# $LLVM_BIN_PATH/llvm-mc"
- << " -arch=x86"
- << " -filetype=obj"
- << " -o=MyObj.o"
- << "\n\n";
- }
IceString MangledName = getContext()->mangleName(getFunctionName());
emitTextHeader(MangledName);
for (CfgNode *Node : Nodes)
diff --git a/src/IceCfgNode.cpp b/src/IceCfgNode.cpp
index 55512e8..4a970ae 100644
--- a/src/IceCfgNode.cpp
+++ b/src/IceCfgNode.cpp
@@ -104,10 +104,6 @@
// Note that this transformation takes the Phi dest variables out of
// SSA form, as there may be assignments to the dest variable in
// multiple blocks.
-//
-// TODO: Defer this pass until after register allocation, then split
-// critical edges, add the assignments, and lower them. This should
-// reduce the amount of shuffling at the end of each block.
void CfgNode::placePhiStores() {
// Find the insertion point.
InstList::iterator InsertionPoint = Insts.end();
@@ -250,14 +246,13 @@
}
}
assert(Found);
- // Repoint a suitable branch instruction's target.
+ // Repoint a suitable branch instruction's target and return.
Found = false;
- for (auto I = Pred->getInsts().rbegin(), E = Pred->getInsts().rend();
- !Found && I != E; ++I) {
- if (!I->isDeleted()) {
- Found = I->repointEdge(this, NewNode);
- }
+ for (Inst &I : reverse_range(Pred->getInsts())) {
+ if (!I.isDeleted() && I.repointEdge(this, NewNode))
+ return NewNode;
}
+ // This should be unreachable, so the assert will fail.
assert(Found);
return NewNode;
}
@@ -533,11 +528,10 @@
SizeT NumVars = Func->getNumVariables();
LivenessBV Live(NumVars);
// Process regular instructions in reverse order.
- // TODO(stichnot): Use llvm::make_range with LLVM 3.5.
- for (auto I = Insts.rbegin(), E = Insts.rend(); I != E; ++I) {
- if (I->isDeleted())
+ for (Inst &I : reverse_range(Insts)) {
+ if (I.isDeleted())
continue;
- I->livenessLightweight(Func, Live);
+ I.livenessLightweight(Func, Live);
}
for (Inst &I : Phis) {
if (I.isDeleted())
@@ -579,10 +573,10 @@
Liveness->getLiveOut(this) = Live;
// Process regular instructions in reverse order.
- for (auto I = Insts.rbegin(), E = Insts.rend(); I != E; ++I) {
- if (I->isDeleted())
+ for (Inst &I : reverse_range(Insts)) {
+ if (I.isDeleted())
continue;
- I->liveness(I->getNumber(), Live, Liveness, LiveBegin, LiveEnd);
+ I.liveness(I.getNumber(), Live, Liveness, LiveBegin, LiveEnd);
}
// Process phis in forward order so that we can override the
// instruction number to be that of the earliest phi instruction in
diff --git a/src/IceDefs.h b/src/IceDefs.h
index 8aafbfd..f5147c6 100644
--- a/src/IceDefs.h
+++ b/src/IceDefs.h
@@ -29,6 +29,7 @@
#include "llvm/ADT/BitVector.h"
#include "llvm/ADT/ilist.h"
#include "llvm/ADT/ilist_node.h"
+#include "llvm/ADT/iterator_range.h"
#include "llvm/ADT/SmallBitVector.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/STLExtras.h"
@@ -157,6 +158,17 @@
typedef llvm::raw_ostream Ostream;
typedef llvm::raw_fd_ostream Fdstream;
+// Reverse range adaptors written in terms of llvm::make_range().
+template <typename T>
+llvm::iterator_range<typename T::const_reverse_iterator>
+reverse_range(const T &Container) {
+ return llvm::make_range(Container.rbegin(), Container.rend());
+}
+template <typename T>
+llvm::iterator_range<typename T::reverse_iterator> reverse_range(T &Container) {
+ return llvm::make_range(Container.rbegin(), Container.rend());
+}
+
} // end of namespace Ice
#endif // SUBZERO_SRC_ICEDEFS_H
diff --git a/src/IceGlobalContext.cpp b/src/IceGlobalContext.cpp
index eb312f9..0a846b9 100644
--- a/src/IceGlobalContext.cpp
+++ b/src/IceGlobalContext.cpp
@@ -130,8 +130,7 @@
IceString TestPrefix, const ClFlags &Flags)
: StrDump(OsDump), StrEmit(OsEmit), VMask(Mask),
ConstPool(new ConstantPool()), Arch(Arch), Opt(Opt),
- TestPrefix(TestPrefix), Flags(Flags), HasEmittedFirstMethod(false),
- RNG(""), ObjectWriter() {
+ TestPrefix(TestPrefix), Flags(Flags), RNG(""), ObjectWriter() {
// Pre-register built-in stack names.
if (ALLOW_DUMP) {
newTimerStackID("Total across all functions");
diff --git a/src/IceGlobalContext.h b/src/IceGlobalContext.h
index 0a0ed75..1b66294 100644
--- a/src/IceGlobalContext.h
+++ b/src/IceGlobalContext.h
@@ -33,7 +33,7 @@
// This class collects rudimentary statistics during translation.
class CodeStats {
CodeStats(const CodeStats &) = delete;
- // CodeStats &operator=(const CodeStats &) = delete;
+ CodeStats &operator=(const CodeStats &) = default;
public:
CodeStats()
@@ -90,15 +90,6 @@
IceString getTestPrefix() const { return TestPrefix; }
IceString mangleName(const IceString &Name) const;
- // The purpose of HasEmitted is to add a header comment at the
- // beginning of assembly code emission, doing it once per file
- // rather than once per function.
- bool testAndSetHasEmittedFirstMethod() {
- bool HasEmitted = HasEmittedFirstMethod;
- HasEmittedFirstMethod = true;
- return HasEmitted;
- }
-
// Manage Constants.
// getConstant*() functions are not const because they might add
// something to the constant pool.
@@ -214,7 +205,6 @@
const OptLevel Opt;
const IceString TestPrefix;
const ClFlags &Flags;
- bool HasEmittedFirstMethod;
RandomNumberGenerator RNG;
std::unique_ptr<ELFObjectWriter> ObjectWriter;
CodeStats StatsFunction;
diff --git a/src/IceLiveness.h b/src/IceLiveness.h
index a542f3e..c287dc2 100644
--- a/src/IceLiveness.h
+++ b/src/IceLiveness.h
@@ -25,40 +25,38 @@
namespace Ice {
-class LivenessNode {
- // TODO: Disable these constructors when Liveness::Nodes is no
- // longer an STL container.
- // LivenessNode(const LivenessNode &) = delete;
- // LivenessNode &operator=(const LivenessNode &) = delete;
-
-public:
- LivenessNode() : NumLocals(0), NumNonDeadPhis(0) {}
- // NumLocals is the number of Variables local to this block.
- SizeT NumLocals;
- // NumNonDeadPhis tracks the number of Phi instructions that
- // Inst::liveness() identified as tentatively live. If
- // NumNonDeadPhis changes from the last liveness pass, then liveness
- // has not yet converged.
- SizeT NumNonDeadPhis;
- // LiveToVarMap maps a liveness bitvector index to a Variable. This
- // is generally just for printing/dumping. The index should be less
- // than NumLocals + Liveness::NumGlobals.
- std::vector<Variable *> LiveToVarMap;
- // LiveIn and LiveOut track the in- and out-liveness of the global
- // variables. The size of each vector is
- // LivenessNode::NumGlobals.
- LivenessBV LiveIn, LiveOut;
- // LiveBegin and LiveEnd track the instruction numbers of the start
- // and end of each variable's live range within this block. The
- // index/key of each element is less than NumLocals +
- // Liveness::NumGlobals.
- LiveBeginEndMap LiveBegin, LiveEnd;
-};
-
class Liveness {
Liveness(const Liveness &) = delete;
Liveness &operator=(const Liveness &) = delete;
+ class LivenessNode {
+ LivenessNode &operator=(const LivenessNode &) = delete;
+
+ public:
+ LivenessNode() : NumLocals(0), NumNonDeadPhis(0) {}
+ LivenessNode(const LivenessNode &) = default;
+ // NumLocals is the number of Variables local to this block.
+ SizeT NumLocals;
+ // NumNonDeadPhis tracks the number of Phi instructions that
+ // Inst::liveness() identified as tentatively live. If
+ // NumNonDeadPhis changes from the last liveness pass, then liveness
+ // has not yet converged.
+ SizeT NumNonDeadPhis;
+ // LiveToVarMap maps a liveness bitvector index to a Variable. This
+ // is generally just for printing/dumping. The index should be less
+ // than NumLocals + Liveness::NumGlobals.
+ std::vector<Variable *> LiveToVarMap;
+ // LiveIn and LiveOut track the in- and out-liveness of the global
+ // variables. The size of each vector is
+ // LivenessNode::NumGlobals.
+ LivenessBV LiveIn, LiveOut;
+ // LiveBegin and LiveEnd track the instruction numbers of the start
+ // and end of each variable's live range within this block. The
+ // index/key of each element is less than NumLocals +
+ // Liveness::NumGlobals.
+ LiveBeginEndMap LiveBegin, LiveEnd;
+ };
+
public:
Liveness(Cfg *Func, LivenessMode Mode)
: Func(Func), Mode(Mode), NumGlobals(0) {}
diff --git a/src/IceOperand.h b/src/IceOperand.h
index db03bba..ee06499 100644
--- a/src/IceOperand.h
+++ b/src/IceOperand.h
@@ -282,12 +282,12 @@
// special value that represents infinite weight, and an addWeight()
// method that ensures that W+infinity=infinity.
class RegWeight {
- // RegWeight(const RegWeight &) = delete;
- // RegWeight &operator=(const RegWeight &) = delete;
public:
RegWeight() : Weight(0) {}
RegWeight(uint32_t Weight) : Weight(Weight) {}
+ RegWeight(const RegWeight &) = default;
+ RegWeight &operator=(const RegWeight &) = default;
const static uint32_t Inf = ~0; // Force regalloc to give a register
const static uint32_t Zero = 0; // Force regalloc NOT to give a register
void addWeight(uint32_t Delta) {
@@ -526,7 +526,6 @@
// VariableTracking tracks the metadata for a single variable. It is
// only meant to be used internally by VariablesMetadata.
class VariableTracking {
- // VariableTracking(const VariableTracking &) = delete;
VariableTracking &operator=(const VariableTracking &) = delete;
public:
@@ -545,6 +544,7 @@
VariableTracking()
: MultiDef(MDS_Unknown), MultiBlock(MBS_Unknown), SingleUseNode(nullptr),
SingleDefNode(nullptr), FirstOrSingleDefinition(nullptr) {}
+ VariableTracking(const VariableTracking &) = default;
MultiDefState getMultiDef() const { return MultiDef; }
MultiBlockState getMultiBlock() const { return MultiBlock; }
const Inst *getFirstDefinition() const;
diff --git a/src/IceRegAlloc.cpp b/src/IceRegAlloc.cpp
index d65b3b6..e7ba50f 100644
--- a/src/IceRegAlloc.cpp
+++ b/src/IceRegAlloc.cpp
@@ -494,9 +494,7 @@
// complexity.
llvm::SmallBitVector PrecoloredUnhandledMask(RegMask.size());
// Note: PrecoloredUnhandledMask is only used for dumping.
- for (auto I = UnhandledPrecolored.rbegin(), E = UnhandledPrecolored.rend();
- I != E; ++I) {
- Variable *Item = *I;
+ for (Variable *Item : reverse_range(UnhandledPrecolored)) {
assert(Item->hasReg());
if (Cur->rangeEndsBefore(Item))
break;
@@ -731,8 +729,8 @@
Str << "\n";
}
Str << "++++++ Unhandled:\n";
- for (auto I = Unhandled.rbegin(), E = Unhandled.rend(); I != E; ++I) {
- dumpLiveRange(*I, Func);
+ for (const Variable *Item : reverse_range(Unhandled)) {
+ dumpLiveRange(Item, Func);
Str << "\n";
}
Str << "++++++ Active:\n";
diff --git a/src/IceTargetLoweringX8632.cpp b/src/IceTargetLoweringX8632.cpp
index b8cbb9d..437f941 100644
--- a/src/IceTargetLoweringX8632.cpp
+++ b/src/IceTargetLoweringX8632.cpp
@@ -13,7 +13,6 @@
//
//===----------------------------------------------------------------------===//
-#include "llvm/ADT/DenseMap.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/MathExtras.h"
@@ -927,7 +926,6 @@
void TargetX8632::addEpilog(CfgNode *Node) {
InstList &Insts = Node->getInsts();
InstList::reverse_iterator RI, E;
- // TODO(stichnot): Use llvm::make_range with LLVM 3.5.
for (RI = Insts.rbegin(), E = Insts.rend(); RI != E; ++RI) {
if (llvm::isa<InstX8632Ret>(*RI))
break;
@@ -4242,9 +4240,9 @@
// register set, and the lowered instruction numbers may be out of
// order, but that can be worked around by renumbering the block
// afterwards if necessary.
- for (auto I = Assignments.rbegin(), E = Assignments.rend(); I != E; ++I) {
+ for (const Inst &I : reverse_range(Assignments)) {
Context.rewind();
- auto Assign = llvm::dyn_cast<InstAssign>(&*I);
+ auto Assign = llvm::dyn_cast<InstAssign>(&I);
Variable *Dest = Assign->getDest();
Operand *Src = Assign->getSrc(0);
Variable *SrcVar = llvm::dyn_cast<Variable>(Src);
diff --git a/src/IceTimerTree.cpp b/src/IceTimerTree.cpp
index 9f2340d..2c912e4 100644
--- a/src/IceTimerTree.cpp
+++ b/src/IceTimerTree.cpp
@@ -141,12 +141,11 @@
void dumpHelper(Ostream &Str, const DumpMapType &Map, double TotalTime) {
if (!ALLOW_DUMP)
return;
- // TODO(stichnot): Use llvm::make_range with LLVM 3.5.
- for (auto I = Map.rbegin(), E = Map.rend(); I != E; ++I) {
+ for (auto &I : reverse_range(Map)) {
char buf[80];
- snprintf(buf, llvm::array_lengthof(buf), " %10.6f (%4.1f%%): ", I->first,
- I->first * 100 / TotalTime);
- Str << buf << I->second << "\n";
+ snprintf(buf, llvm::array_lengthof(buf), " %10.6f (%4.1f%%): ", I.first,
+ I.first * 100 / TotalTime);
+ Str << buf << I.second << "\n";
}
}
diff --git a/src/IceTimerTree.h b/src/IceTimerTree.h
index bc38245..bf122fd 100644
--- a/src/IceTimerTree.h
+++ b/src/IceTimerTree.h
@@ -19,34 +19,31 @@
namespace Ice {
-class TimerTreeNode;
-
-// Timer tree index type
-typedef std::vector<TimerTreeNode>::size_type TTindex;
-
-// TimerTreeNode represents an interior or leaf node in the call tree.
-// It contains a list of children, a pointer to its parent, and the
-// timer ID for the node. It also holds the cumulative time spent at
-// this node and below. The children are always at a higher index in
-// the TimerTreeNode::Nodes array, and the parent is always at a lower
-// index.
-class TimerTreeNode {
- // TimerTreeNode(const TimerTreeNode &) = delete;
- TimerTreeNode &operator=(const TimerTreeNode &) = delete;
-
-public:
- TimerTreeNode() : Parent(0), Interior(0), Time(0), UpdateCount(0) {}
- std::vector<TTindex> Children; // indexed by TimerIdT
- TTindex Parent;
- TimerIdT Interior;
- double Time;
- size_t UpdateCount;
-};
-
class TimerStack {
- // TimerStack(const TimerStack &) = delete;
TimerStack &operator=(const TimerStack &) = delete;
+ // Timer tree index type
+ typedef std::vector<class TimerTreeNode>::size_type TTindex;
+
+ // TimerTreeNode represents an interior or leaf node in the call tree.
+ // It contains a list of children, a pointer to its parent, and the
+ // timer ID for the node. It also holds the cumulative time spent at
+ // this node and below. The children are always at a higher index in
+ // the TimerTreeNode::Nodes array, and the parent is always at a lower
+ // index.
+ class TimerTreeNode {
+ TimerTreeNode &operator=(const TimerTreeNode &) = delete;
+
+ public:
+ TimerTreeNode() : Parent(0), Interior(0), Time(0), UpdateCount(0) {}
+ TimerTreeNode(const TimerTreeNode &) = default;
+ std::vector<TTindex> Children; // indexed by TimerIdT
+ TTindex Parent;
+ TimerIdT Interior;
+ double Time;
+ size_t UpdateCount;
+ };
+
public:
enum TimerTag {
#define X(tag) TT_##tag,
@@ -55,6 +52,7 @@
TT__num
};
TimerStack(const IceString &Name);
+ TimerStack(const TimerStack &) = default;
TimerIdT getTimerID(const IceString &Name);
void setName(const IceString &NewName) { Name = NewName; }
void push(TimerIdT ID);
diff --git a/src/IceTypes.h b/src/IceTypes.h
index f3ad672..2b4c2cd 100644
--- a/src/IceTypes.h
+++ b/src/IceTypes.h
@@ -112,10 +112,7 @@
}
/// Models a type signature for a function.
-/// TODO(kschimpf): Consider using arena memory allocation for
-/// the contents of type signatures.
class FuncSigType {
- // FuncSigType(const FuncSigType &Ty) = delete;
FuncSigType &operator=(const FuncSigType &Ty) = delete;
public:
typedef std::vector<Type> ArgListType;
@@ -123,6 +120,7 @@
// Creates a function signature type with the given return type.
// Parameter types should be added using calls to appendArgType.
FuncSigType() : ReturnType(IceType_void) {}
+ FuncSigType(const FuncSigType &Ty) = default;
void appendArgType(Type ArgType) { ArgList.push_back(ArgType); }
diff --git a/src/PNaClTranslator.cpp b/src/PNaClTranslator.cpp
index ac4c697..9be47f5 100644
--- a/src/PNaClTranslator.cpp
+++ b/src/PNaClTranslator.cpp
@@ -49,13 +49,13 @@
// Use methods setAsSimpleType and setAsFuncSigType to define
// the extended type.
class ExtendedType {
- // ExtendedType(const ExtendedType &Ty) = delete;
ExtendedType &operator=(const ExtendedType &Ty) = delete;
public:
/// Discriminator for LLVM-style RTTI.
enum TypeKind { Undefined, Simple, FuncSig };
ExtendedType() : Kind(Undefined) {}
+ ExtendedType(const ExtendedType &Ty) = default;
virtual ~ExtendedType() {}
