Subzero: Fix a bug in register allocator overlap computation.
When the register allocator decides whether to allow the candidate's live range to overlap its preferred variable's live range (and share their register), it needs to consider whether any redefinitions in one variable occur within the live range of the other variable, in which case overlap should not be allowed.
There was a bug in the API for iterating over the defining instructions for a variable, in which the earliest definition might be ignored in some cases. This came from the fact that the first definition and latter definitions are split apart for translation speed reasons, and a particular API is needed for finding an unambiguous first definition, which is possible when all definitions are within a single block but not so possible when definitions cross block boundaries. (This only happens for the simple phi lowering.)
Since both semantics are needed, a separate API is added to support both.
For spec2k, the asm output is identical to before, so this changes nothing. When translating spec2k with "-O2 -phi-edge-split=0", there is a single minor difference in ammp that actually looks legit in both cases.
However, when testing an upcoming CL, -phi-edge-split=0 triggered the bug, causing gcc and crafty to fail with incorrect output.
This CL also fixes some minor issues, and adds dump output of the instruction definition list when available.
BUG= none
R=jpp@chromium.org
Review URL: https://codereview.chromium.org/1381563004 .
diff --git a/src/IceOperand.cpp b/src/IceOperand.cpp
index 125c692..66321c2 100644
--- a/src/IceOperand.cpp
+++ b/src/IceOperand.cpp
@@ -213,15 +213,16 @@
// omit all uses of the variable if markDef() and markUse() both use this
// optimization.
assert(Node);
-// Verify that instructions are added in increasing order.
-#ifndef NDEBUG
- if (TrackingKind == VMK_All) {
- const Inst *LastInstruction =
- Definitions.empty() ? FirstOrSingleDefinition : Definitions.back();
- assert(LastInstruction == nullptr ||
- Instr->getNumber() >= LastInstruction->getNumber());
+ // Verify that instructions are added in increasing order.
+ if (BuildDefs::asserts()) {
+ if (TrackingKind == VMK_All) {
+ const Inst *LastInstruction =
+ Definitions.empty() ? FirstOrSingleDefinition : Definitions.back();
+ (void)LastInstruction;
+ assert(LastInstruction == nullptr ||
+ Instr->getNumber() >= LastInstruction->getNumber());
+ }
}
-#endif
constexpr bool IsImplicit = false;
markUse(TrackingKind, Instr, Node, IsImplicit);
if (TrackingKind == VMK_Uses)
@@ -258,7 +259,7 @@
}
}
-const Inst *VariableTracking::getFirstDefinition() const {
+const Inst *VariableTracking::getFirstDefinitionSingleBlock() const {
switch (MultiDef) {
case MDS_Unknown:
case MDS_MultiDefMultiBlock:
@@ -284,6 +285,19 @@
return nullptr;
}
+const Inst *VariableTracking::getFirstDefinition() const {
+ switch (MultiDef) {
+ case MDS_Unknown:
+ return nullptr;
+ case MDS_MultiDefMultiBlock:
+ case MDS_SingleDef:
+ case MDS_MultiDefSingleBlock:
+ assert(FirstOrSingleDefinition);
+ return FirstOrSingleDefinition;
+ }
+ return nullptr;
+}
+
void VariablesMetadata::init(MetadataKind TrackingKind) {
TimerMarker T(TimerStack::TT_vmetadata, Func);
Kind = TrackingKind;
@@ -303,7 +317,7 @@
}
void VariablesMetadata::addNode(CfgNode *Node) {
- if (Func->getNumVariables() >= Metadata.size())
+ if (Func->getNumVariables() > Metadata.size())
Metadata.resize(Func->getNumVariables());
for (Inst &I : Node->getPhis()) {
@@ -364,12 +378,13 @@
return Metadata[VarNum].getMultiBlock() != VariableTracking::MBS_SingleBlock;
}
-const Inst *VariablesMetadata::getFirstDefinition(const Variable *Var) const {
+const Inst *
+VariablesMetadata::getFirstDefinitionSingleBlock(const Variable *Var) const {
assert(Kind != VMK_Uses);
if (!isTracked(Var))
return nullptr; // conservative answer
SizeT VarNum = Var->getIndex();
- return Metadata[VarNum].getFirstDefinition();
+ return Metadata[VarNum].getFirstDefinitionSingleBlock();
}
const Inst *VariablesMetadata::getSingleDefinition(const Variable *Var) const {
@@ -380,6 +395,14 @@
return Metadata[VarNum].getSingleDefinition();
}
+const Inst *VariablesMetadata::getFirstDefinition(const Variable *Var) const {
+ assert(Kind != VMK_Uses);
+ if (!isTracked(Var))
+ return nullptr; // conservative answer
+ SizeT VarNum = Var->getIndex();
+ return Metadata[VarNum].getFirstDefinition();
+}
+
const InstDefList &
VariablesMetadata::getLatterDefinitions(const Variable *Var) const {
assert(Kind == VMK_All);