Commit fe62f0a2 by Jim Stichnoth

Subzero: Allow deeper levels of variable splitting.

This fixes some existing problems with the Variable::LinkedTo splitting/linking mechanism. The problem was that if B is linked to A, and B needs a stack slot, but A doesn't get a stack slot, B's stack offset would never get initialized. This could happen if A ends up with no explicit references in the code, or A's live range gets truncated such that it actually has a register while B doesn't. It gets even more complicated if you have a link chain like A<--B<--C<--D etc. where some of them have stack slots (which should ultimately all be the same slot) and some don't. The solution here is that if B is linked to the root A, and B has a stack slot but A doesn't, we can do a tree rotation so that B is the new root and A links to B. In addition, we initialize Variable::StackOffset to an invalid value and always make sure a value used is valid. Earlier attempts at extending the variable splitting would sometimes silently fail because the default StackOffset value of 0 ended up being used. BUG= none R=jpp@chromium.org Review URL: https://codereview.chromium.org/2116213002 .
parent 3f97afb1
...@@ -1577,7 +1577,7 @@ void Cfg::emit() { ...@@ -1577,7 +1577,7 @@ void Cfg::emit() {
emitTextHeader(FunctionName, Ctx, Asm); emitTextHeader(FunctionName, Ctx, Asm);
if (getFlags().getDecorateAsm()) { if (getFlags().getDecorateAsm()) {
for (Variable *Var : getVariables()) { for (Variable *Var : getVariables()) {
if (Var->getStackOffset() && !Var->isRematerializable()) { if (Var->hasStackOffset() && !Var->isRematerializable()) {
Str << "\t" << Var->getSymbolicStackOffset() << " = " Str << "\t" << Var->getSymbolicStackOffset() << " = "
<< Var->getStackOffset() << "\n"; << Var->getStackOffset() << "\n";
} }
......
...@@ -1094,9 +1094,15 @@ bool checkForRedundantAssign(const Variable *Dest, const Operand *Source) { ...@@ -1094,9 +1094,15 @@ bool checkForRedundantAssign(const Variable *Dest, const Operand *Source) {
// upper 32 bits of rax. We need to recognize and preserve these. // upper 32 bits of rax. We need to recognize and preserve these.
return true; return true;
} }
if (!Dest->hasReg() && !SrcVar->hasReg() && if (!Dest->hasReg() && !SrcVar->hasReg()) {
Dest->getStackOffset() == SrcVar->getStackOffset()) if (!Dest->hasStackOffset() || !SrcVar->hasStackOffset()) {
return false;
}
if (Dest->getStackOffset() != SrcVar->getStackOffset()) {
return false;
}
return true; return true;
}
return false; return false;
} }
......
...@@ -554,12 +554,14 @@ void Variable::dump(const Cfg *Func, Ostream &Str) const { ...@@ -554,12 +554,14 @@ void Variable::dump(const Cfg *Func, Ostream &Str) const {
hasReg() ? getBaseRegNum() : Func->getTarget()->getFrameOrStackReg(); hasReg() ? getBaseRegNum() : Func->getTarget()->getFrameOrStackReg();
Str << "[" Str << "["
<< Func->getTarget()->getRegName(BaseRegisterNumber, IceType_i32); << Func->getTarget()->getRegName(BaseRegisterNumber, IceType_i32);
if (hasStackOffset()) {
int32_t Offset = getStackOffset(); int32_t Offset = getStackOffset();
if (Offset) { if (Offset) {
if (Offset > 0) if (Offset > 0)
Str << "+"; Str << "+";
Str << Offset; Str << Offset;
} }
}
Str << "]"; Str << "]";
} }
} }
......
...@@ -696,7 +696,11 @@ public: ...@@ -696,7 +696,11 @@ public:
return IgnoreLiveness || IsRematerializable; return IgnoreLiveness || IsRematerializable;
} }
int32_t getStackOffset() const { return StackOffset; } bool hasStackOffset() const { return StackOffset != InvalidStackOffset; }
int32_t getStackOffset() const {
assert(hasStackOffset());
return StackOffset;
}
void setStackOffset(int32_t Offset) { StackOffset = Offset; } void setStackOffset(int32_t Offset) { StackOffset = Offset; }
/// Returns the variable's stack offset in symbolic form, to improve /// Returns the variable's stack offset in symbolic form, to improve
/// readability in DecorateAsm mode. /// readability in DecorateAsm mode.
...@@ -773,21 +777,17 @@ public: ...@@ -773,21 +777,17 @@ public:
/// Return reg num of base register, if different from stack/frame register. /// Return reg num of base register, if different from stack/frame register.
virtual RegNumT getBaseRegNum() const { return RegNumT(); } virtual RegNumT getBaseRegNum() const { return RegNumT(); }
void setLinkedTo(const Variable *Var) { /// Access the LinkedTo field.
// If B is linked to A, and we now want to link C to B, we instead link C to void setLinkedTo(Variable *Var) { LinkedTo = Var; }
// A so that we have one root (A) and all leaves (B, C) link directly to the Variable *getLinkedTo() const { return LinkedTo; }
// root. /// Follow the LinkedTo chain up to the furthest ancestor.
if (Var->getLinkedTo() != nullptr) { Variable *getLinkedToRoot() const {
Var = Var->LinkedTo; Variable *Root = LinkedTo;
assert(Var->LinkedTo == nullptr); if (Root == nullptr)
} return nullptr;
LinkedTo = Var; while (Root->LinkedTo != nullptr)
} Root = Root->LinkedTo;
const Variable *getLinkedTo() const { return Root;
// Make sure a leaf links directly to the root.
if (LinkedTo != nullptr)
assert(LinkedTo->LinkedTo == nullptr);
return LinkedTo;
} }
static bool classof(const Operand *Operand) { static bool classof(const Operand *Operand) {
...@@ -825,15 +825,16 @@ protected: ...@@ -825,15 +825,16 @@ protected:
RegNumT RegNum; RegNumT RegNum;
/// RegNumTmp is the tentative assignment during register allocation. /// RegNumTmp is the tentative assignment during register allocation.
RegNumT RegNumTmp; RegNumT RegNumTmp;
static constexpr int32_t InvalidStackOffset = -1;
/// StackOffset is the canonical location on stack (only if /// StackOffset is the canonical location on stack (only if
/// RegNum.hasNoValue() || IsArgument). /// RegNum.hasNoValue() || IsArgument).
int32_t StackOffset = 0; int32_t StackOffset = InvalidStackOffset;
LiveRange Live; LiveRange Live;
/// VarsReal (and Operand::Vars) are set up such that Vars[0] == this. /// VarsReal (and Operand::Vars) are set up such that Vars[0] == this.
Variable *VarsReal[1]; Variable *VarsReal[1];
/// This Variable may be "linked" to another Variable, such that if neither /// This Variable may be "linked" to another Variable, such that if neither
/// Variable gets a register, they are guaranteed to share a stack location. /// Variable gets a register, they are guaranteed to share a stack location.
const Variable *LinkedTo = nullptr; Variable *LinkedTo = nullptr;
}; };
// Variable64On32 represents a 64-bit variable on a 32-bit architecture. In // Variable64On32 represents a 64-bit variable on a 32-bit architecture. In
......
...@@ -564,6 +564,16 @@ void TargetLowering::sortVarsByAlignment(VarList &Dest, ...@@ -564,6 +564,16 @@ void TargetLowering::sortVarsByAlignment(VarList &Dest,
}); });
} }
namespace {
bool mightHaveStackSlot(const Variable *Var, const BitVector &IsVarReferenced) {
if (!IsVarReferenced[Var->getIndex()])
return false;
if (Var->hasReg())
return false;
return true;
}
} // end of anonymous namespace
void TargetLowering::getVarStackSlotParams( void TargetLowering::getVarStackSlotParams(
VarList &SortedSpilledVariables, SmallBitVector &RegsUsed, VarList &SortedSpilledVariables, SmallBitVector &RegsUsed,
size_t *GlobalsSize, size_t *SpillAreaSizeBytes, size_t *GlobalsSize, size_t *SpillAreaSizeBytes,
...@@ -583,6 +593,30 @@ void TargetLowering::getVarStackSlotParams( ...@@ -583,6 +593,30 @@ void TargetLowering::getVarStackSlotParams(
} }
} }
// Find each variable Var where:
// - Var is actively referenced
// - Var does not have a register
// - Var's furthest ancestor through LinkedTo: Root
// - Root has no active references, or has a register
//
// When any such Var is found, rotate the LinkedTo tree by swapping
// Var->LinkedTo and Root->LinkedTo. This ensures that when Var needs a stack
// slot, either its LinkedTo field is nullptr, or Var->getLinkedToRoot()
// returns a variable with a stack slot.
for (Variable *Var : Func->getVariables()) {
if (!mightHaveStackSlot(Var, IsVarReferenced))
continue;
if (Variable *Root = Var->getLinkedToRoot()) {
assert(Root->getLinkedTo() == nullptr);
if (mightHaveStackSlot(Root, IsVarReferenced)) {
// Found a "safe" root, no need to rotate the tree.
continue;
}
Var->setLinkedTo(nullptr);
Root->setLinkedTo(Var);
}
}
// If SimpleCoalescing is false, each variable without a register gets its // If SimpleCoalescing is false, each variable without a register gets its
// own unique stack slot, which leads to large stack frames. If // own unique stack slot, which leads to large stack frames. If
// SimpleCoalescing is true, then each "global" variable without a register // SimpleCoalescing is true, then each "global" variable without a register
......
...@@ -468,9 +468,10 @@ OperandMIPS32Mem *TargetMIPS32::formMemoryOperand(Operand *Operand, Type Ty) { ...@@ -468,9 +468,10 @@ OperandMIPS32Mem *TargetMIPS32::formMemoryOperand(Operand *Operand, Type Ty) {
// to work with. MIPS always requires a base register, so just use that to // to work with. MIPS always requires a base register, so just use that to
// hold the operand. // hold the operand.
auto *Base = llvm::cast<Variable>(legalize(Operand, Legal_Reg)); auto *Base = llvm::cast<Variable>(legalize(Operand, Legal_Reg));
const int32_t Offset = Base->hasStackOffset() ? Base->getStackOffset() : 0;
return OperandMIPS32Mem::create( return OperandMIPS32Mem::create(
Func, Ty, Base, llvm::cast<ConstantInteger32>( Func, Ty, Base,
Ctx->getConstantInt32(Base->getStackOffset()))); llvm::cast<ConstantInteger32>(Ctx->getConstantInt32(Offset)));
} }
void TargetMIPS32::emitVariable(const Variable *Var) const { void TargetMIPS32::emitVariable(const Variable *Var) const {
......
...@@ -944,18 +944,13 @@ void TargetX86Base<TraitsType>::emitVariable(const Variable *Var) const { ...@@ -944,18 +944,13 @@ void TargetX86Base<TraitsType>::emitVariable(const Variable *Var) const {
auto BaseRegNum = Var->getBaseRegNum(); auto BaseRegNum = Var->getBaseRegNum();
if (BaseRegNum.hasNoValue()) if (BaseRegNum.hasNoValue())
BaseRegNum = getFrameOrStackReg(); BaseRegNum = getFrameOrStackReg();
// Print in the form "Offset(%reg)", taking care that:
// - Offset is never printed when it is 0
const bool DecorateAsm = getFlags().getDecorateAsm(); // Print in the form "Offset(%reg)", omitting Offset when it is 0.
// Only print Offset when it is nonzero, regardless of DecorateAsm. if (getFlags().getDecorateAsm()) {
if (Offset) {
if (DecorateAsm) {
Str << Var->getSymbolicStackOffset(); Str << Var->getSymbolicStackOffset();
} else { } else if (Offset != 0) {
Str << Offset; Str << Offset;
} }
}
const Type FrameSPTy = Traits::WordType; const Type FrameSPTy = Traits::WordType;
Str << "(%" << getRegName(BaseRegNum, FrameSPTy) << ")"; Str << "(%" << getRegName(BaseRegNum, FrameSPTy) << ")";
} }
...@@ -1211,9 +1206,9 @@ void TargetX86Base<TraitsType>::addProlog(CfgNode *Node) { ...@@ -1211,9 +1206,9 @@ void TargetX86Base<TraitsType>::addProlog(CfgNode *Node) {
// Assign stack offsets to variables that have been linked to spilled // Assign stack offsets to variables that have been linked to spilled
// variables. // variables.
for (Variable *Var : VariablesLinkedToSpillSlots) { for (Variable *Var : VariablesLinkedToSpillSlots) {
const Variable *Linked = Var->getLinkedTo(); const Variable *Root = Var->getLinkedToRoot();
assert(Linked != nullptr); assert(Root != nullptr);
Var->setStackOffset(Linked->getStackOffset()); Var->setStackOffset(Root->getStackOffset());
} }
this->HasComputedFrame = true; this->HasComputedFrame = true;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment