Skip to content

S
swiftshader
Commit 1fb030c6 by Jim Stichnoth
Options

Subzero: Various fixes in preparation for x86-32 register aliasing.

1. Helper function sameVarOrReg() also needs to return true if the two physical registers alias or overlap. Otherwise advanced phi lowering may pick an incorrect ordering. 2. With -asm-verbose, redundant truncation assignments expressed as _mov instructions, like "mov cl, ecx", need to have their register use counts updated properly, so that the LIVEEND= annotations are correct. 3. The register allocator should consider suitably typed aliases when choosing a register preference. 4. When evicting a variable, the register allocator should decrement the use count of all aliases. 5. When saving/restoring callee-save registers in the prolog/epilog, map each register to its "canonical" register (e.g. %bl --> %ebx) and make sure each canonical register is only considered once. 6. Remove some unnecessary Variable::setMustHaveReg() calls. 7. When assigning bool results as a constant 0 or 1, use an 8-bit constant instead of 32-bit so that only the 8-bit register gets assigned. BUG= none TEST= make check, plus spec2k -asm-verbose output is unchanged R=kschimpf@google.com Review URL: https://codereview.chromium.org/1405643003 .
parent 5c87542a
Showing with 67 additions and 32 deletions
src/IceCfgNode.cpp
...@@ -299,14 +299,29 @@ CfgNode *CfgNode::splitIncomingEdge(CfgNode *Pred, SizeT EdgeIndex) { ...@@ -299,14 +299,29 @@ CfgNode *CfgNode::splitIncomingEdge(CfgNode *Pred, SizeT EdgeIndex) {
namespace { namespace {
// Helper function used by advancedPhiLowering(). // Helper function used by advancedPhiLowering().
bool sameVarOrReg(const Variable *Var, const Operand *Opnd) { bool sameVarOrReg(TargetLowering *Target, const Variable *Var1,
if (Var == Opnd) const Operand *Opnd) {
if (Var1 == Opnd)
return true; return true;
if (const auto Var2 = llvm::dyn_cast<Variable>(Opnd)) { const auto Var2 = llvm::dyn_cast<Variable>(Opnd);
if (Var->hasReg() && Var->getRegNum() == Var2->getRegNum()) if (Var2 == nullptr)
return true; return false;
}
return false; // If either operand lacks a register, they cannot be the same.
if (!Var1->hasReg())
return false;
if (!Var2->hasReg())
return false;
int32_t RegNum1 = Var1->getRegNum();
int32_t RegNum2 = Var2->getRegNum();
// Quick common-case check.
if (RegNum1 == RegNum2)
return true;
assert(Target->getAliasesForRegister(RegNum1)[RegNum2] ==
Target->getAliasesForRegister(RegNum2)[RegNum1]);
return Target->getAliasesForRegister(RegNum1)[RegNum2];
} }
} // end of anonymous namespace } // end of anonymous namespace
...@@ -383,6 +398,7 @@ void CfgNode::advancedPhiLowering() { ...@@ -383,6 +398,7 @@ void CfgNode::advancedPhiLowering() {
if (NumPhis == 0) if (NumPhis == 0)
return; return;
TargetLowering *Target = Func->getTarget();
SizeT InEdgeIndex = 0; SizeT InEdgeIndex = 0;
for (CfgNode *Pred : InEdges) { for (CfgNode *Pred : InEdges) {
CfgNode *Split = splitIncomingEdge(Pred, InEdgeIndex++); CfgNode *Split = splitIncomingEdge(Pred, InEdgeIndex++);
...@@ -397,7 +413,7 @@ void CfgNode::advancedPhiLowering() { ...@@ -397,7 +413,7 @@ void CfgNode::advancedPhiLowering() {
Desc[I].NumPred = 0; Desc[I].NumPred = 0;
// Cherry-pick any trivial assignments, so that they don't contribute to // Cherry-pick any trivial assignments, so that they don't contribute to
// the running complexity of the topological sort. // the running complexity of the topological sort.
if (sameVarOrReg(Dest, Src)) { if (sameVarOrReg(Target, Dest, Src)) {
Desc[I].Processed = true; Desc[I].Processed = true;
--Remaining; --Remaining;
if (Dest != Src) if (Dest != Src)
...@@ -420,10 +436,10 @@ void CfgNode::advancedPhiLowering() { ...@@ -420,10 +436,10 @@ void CfgNode::advancedPhiLowering() {
if (I != J) { if (I != J) {
// There shouldn't be two Phis with the same Dest variable or // There shouldn't be two Phis with the same Dest variable or
// register. // register.
assert(!sameVarOrReg(Dest, Desc[J].Dest)); assert(!sameVarOrReg(Target, Dest, Desc[J].Dest));
} }
const Operand *Src = Desc[J].Src; const Operand *Src = Desc[J].Src;
if (sameVarOrReg(Dest, Src)) if (sameVarOrReg(Target, Dest, Src))
++Desc[I].NumPred; ++Desc[I].NumPred;
} }
} }
...@@ -473,7 +489,7 @@ void CfgNode::advancedPhiLowering() { ...@@ -473,7 +489,7 @@ void CfgNode::advancedPhiLowering() {
assert(Desc[BestIndex].NumPred <= 1); assert(Desc[BestIndex].NumPred <= 1);
Variable *Dest = Desc[BestIndex].Dest; Variable *Dest = Desc[BestIndex].Dest;
Operand *Src = Desc[BestIndex].Src; Operand *Src = Desc[BestIndex].Src;
assert(!sameVarOrReg(Dest, Src)); assert(!sameVarOrReg(Target, Dest, Src));
// Break a cycle by introducing a temporary. // Break a cycle by introducing a temporary.
if (Desc[BestIndex].NumPred) { if (Desc[BestIndex].NumPred) {
bool Found = false; bool Found = false;
...@@ -484,7 +500,7 @@ void CfgNode::advancedPhiLowering() { ...@@ -484,7 +500,7 @@ void CfgNode::advancedPhiLowering() {
if (Desc[J].Processed) if (Desc[J].Processed)
continue; continue;
Operand *OtherSrc = Desc[J].Src; Operand *OtherSrc = Desc[J].Src;
if (Desc[J].NumPred && sameVarOrReg(Dest, OtherSrc)) { if (Desc[J].NumPred && sameVarOrReg(Target, Dest, OtherSrc)) {
SizeT VarNum = Func->getNumVariables(); SizeT VarNum = Func->getNumVariables();
Variable *Tmp = Func->makeVariable(OtherSrc->getType()); Variable *Tmp = Func->makeVariable(OtherSrc->getType());
if (BuildDefs::dump()) if (BuildDefs::dump())
...@@ -505,7 +521,7 @@ void CfgNode::advancedPhiLowering() { ...@@ -505,7 +521,7 @@ void CfgNode::advancedPhiLowering() {
for (size_t I = 0; I < NumPhis; ++I) { for (size_t I = 0; I < NumPhis; ++I) {
if (Desc[I].Processed) if (Desc[I].Processed)
continue; continue;
if (sameVarOrReg(Var, Desc[I].Dest)) { if (sameVarOrReg(Target, Var, Desc[I].Dest)) {
if (--Desc[I].NumPred == 0) if (--Desc[I].NumPred == 0)
Desc[I].Weight += WeightNoPreds; Desc[I].Weight += WeightNoPreds;
} }
...@@ -1030,8 +1046,11 @@ void CfgNode::emit(Cfg *Func) const { ...@@ -1030,8 +1046,11 @@ void CfgNode::emit(Cfg *Func) const {
// That normally would have happened as part of emitLiveRangesEnded(), // That normally would have happened as part of emitLiveRangesEnded(),
// but that isn't called for redundant assignments. // but that isn't called for redundant assignments.
Variable *Dest = I.getDest(); Variable *Dest = I.getDest();
if (DecorateAsm && Dest->hasReg() && !I.isLastUse(I.getSrc(0))) if (DecorateAsm && Dest->hasReg()) {
++LiveRegCount[Dest->getRegNum()]; ++LiveRegCount[Dest->getRegNum()];
if (I.isLastUse(I.getSrc(0)))
--LiveRegCount[llvm::cast<Variable>(I.getSrc(0))->getRegNum()];
}
continue; continue;
} }
I.emit(Func); I.emit(Func);
......
src/IceInst.cpp
...@@ -947,7 +947,7 @@ void InstTarget::dump(const Cfg *Func) const { ...@@ -947,7 +947,7 @@ void InstTarget::dump(const Cfg *Func) const {
} }
bool checkForRedundantAssign(const Variable *Dest, const Operand *Source) { bool checkForRedundantAssign(const Variable *Dest, const Operand *Source) {
const auto SrcVar = llvm::dyn_cast<const Variable>(Source); const auto *SrcVar = llvm::dyn_cast<const Variable>(Source);
if (!SrcVar) if (!SrcVar)
return false; return false;
if (Dest->hasReg() && Dest->getRegNum() == SrcVar->getRegNum()) { if (Dest->hasReg() && Dest->getRegNum() == SrcVar->getRegNum()) {
......
src/IceRegAlloc.cpp
...@@ -501,9 +501,10 @@ void LinearScan::findRegisterPreference(IterationState &Iter) { ...@@ -501,9 +501,10 @@ void LinearScan::findRegisterPreference(IterationState &Iter) {
// try to prefer the stack pointer as a result of the stacksave // try to prefer the stack pointer as a result of the stacksave
// intrinsic. // intrinsic.
if (SrcVar->hasRegTmp()) { if (SrcVar->hasRegTmp()) {
const int32_t SrcReg = SrcVar->getRegNumTmp(); const llvm::SmallBitVector &Aliases =
const bool IsAliasAvailable = *RegAliases[SrcVar->getRegNumTmp()];
(Iter.RegMask & *RegAliases[SrcReg]).any(); const int32_t SrcReg = (Iter.RegMask & Aliases).find_first();
const bool IsAliasAvailable = (SrcReg != -1);
if (IsAliasAvailable) { if (IsAliasAvailable) {
if (FindOverlap && !Iter.Free[SrcReg]) { if (FindOverlap && !Iter.Free[SrcReg]) {
// Don't bother trying to enable AllowOverlap if the register is // Don't bother trying to enable AllowOverlap if the register is
...@@ -694,8 +695,12 @@ void LinearScan::handleNoFreeRegisters(IterationState &Iter) { ...@@ -694,8 +695,12 @@ void LinearScan::handleNoFreeRegisters(IterationState &Iter) {
int32_t RegNum = Item->getRegNumTmp(); int32_t RegNum = Item->getRegNumTmp();
if (Aliases[RegNum]) { if (Aliases[RegNum]) {
dumpLiveRangeTrace("Evicting A ", Item); dumpLiveRangeTrace("Evicting A ", Item);
--RegUses[RegNum]; const llvm::SmallBitVector &Aliases = *RegAliases[RegNum];
assert(RegUses[RegNum] >= 0); for (int32_t RegAlias = Aliases.find_first(); RegAlias >= 0;
RegAlias = Aliases.find_next(RegAlias)) {
--RegUses[RegAlias];
assert(RegUses[RegAlias] >= 0);
}
Item->setRegNumTmp(Variable::NoRegister); Item->setRegNumTmp(Variable::NoRegister);
moveItem(Active, Index, Handled); moveItem(Active, Index, Handled);
Evicted.push_back(Item); Evicted.push_back(Item);
......
src/IceTargetLoweringX8632.cpp
...@@ -436,8 +436,16 @@ void TargetX8632::addProlog(CfgNode *Node) { ...@@ -436,8 +436,16 @@ void TargetX8632::addProlog(CfgNode *Node) {
// Add push instructions for preserved registers. // Add push instructions for preserved registers.
uint32_t NumCallee = 0; uint32_t NumCallee = 0;
size_t PreservedRegsSizeBytes = 0; size_t PreservedRegsSizeBytes = 0;
llvm::SmallBitVector Pushed(CalleeSaves.size());
for (SizeT i = 0; i < CalleeSaves.size(); ++i) { for (SizeT i = 0; i < CalleeSaves.size(); ++i) {
// SizeT Canonical = RegX8632::getCanonicalReg(i); // TODO(stichnot)
SizeT Canonical = i;
if (CalleeSaves[i] && RegsUsed[i]) { if (CalleeSaves[i] && RegsUsed[i]) {
Pushed[Canonical] = true;
}
}
for (SizeT i = 0; i < Pushed.size(); ++i) {
if (Pushed[i]) {
++NumCallee; ++NumCallee;
PreservedRegsSizeBytes += typeWidthInBytes(IceType_i32); PreservedRegsSizeBytes += typeWidthInBytes(IceType_i32);
_push(getPhysicalRegister(i)); _push(getPhysicalRegister(i));
...@@ -601,11 +609,19 @@ void TargetX8632::addEpilog(CfgNode *Node) { ...@@ -601,11 +609,19 @@ void TargetX8632::addEpilog(CfgNode *Node) {
// Add pop instructions for preserved registers. // Add pop instructions for preserved registers.
llvm::SmallBitVector CalleeSaves = llvm::SmallBitVector CalleeSaves =
getRegisterSet(RegSet_CalleeSave, RegSet_None); getRegisterSet(RegSet_CalleeSave, RegSet_None);
llvm::SmallBitVector Popped(CalleeSaves.size());
for (SizeT i = 0; i < CalleeSaves.size(); ++i) { for (SizeT i = 0; i < CalleeSaves.size(); ++i) {
SizeT j = CalleeSaves.size() - i - 1; // SizeT Canonical = RegX8632::getCanonicalReg(i); // TODO(stichnot)
SizeT Canonical = i;
if (CalleeSaves[i] && RegsUsed[i]) {
Popped[Canonical] = true;
}
}
for (SizeT i = 0; i < Popped.size(); ++i) {
SizeT j = Popped.size() - i - 1;
if (j == Traits::RegisterSet::Reg_ebp && IsEbpBasedFrame) if (j == Traits::RegisterSet::Reg_ebp && IsEbpBasedFrame)
continue; continue;
if (CalleeSaves[j] && RegsUsed[j]) { if (Popped[j]) {
_pop(getPhysicalRegister(j)); _pop(getPhysicalRegister(j));
} }
} }
......
src/IceTargetLoweringX86BaseImpl.h
...@@ -2136,7 +2136,6 @@ void TargetX86Base<Machine>::lowerCast(const InstCast *Inst) { ...@@ -2136,7 +2136,6 @@ void TargetX86Base<Machine>::lowerCast(const InstCast *Inst) {
T_1 = makeReg(IceType_i32); T_1 = makeReg(IceType_i32);
} }
// cvt() requires its integer argument to be a GPR. // cvt() requires its integer argument to be a GPR.
T_1->setMustHaveReg();
Variable *T_2 = makeReg(Dest->getType()); Variable *T_2 = makeReg(Dest->getType());
_cvt(T_1, Src0RM, Traits::Insts::Cvt::Tss2si); _cvt(T_1, Src0RM, Traits::Insts::Cvt::Tss2si);
_mov(T_2, T_1); // T_1 and T_2 may have different integer types _mov(T_2, T_1); // T_1 and T_2 may have different integer types
...@@ -2185,7 +2184,6 @@ void TargetX86Base<Machine>::lowerCast(const InstCast *Inst) { ...@@ -2185,7 +2184,6 @@ void TargetX86Base<Machine>::lowerCast(const InstCast *Inst) {
assert(Dest->getType() != IceType_i32); assert(Dest->getType() != IceType_i32);
T_1 = makeReg(IceType_i32); T_1 = makeReg(IceType_i32);
} }
T_1->setMustHaveReg();
Variable *T_2 = makeReg(Dest->getType()); Variable *T_2 = makeReg(Dest->getType());
_cvt(T_1, Src0RM, Traits::Insts::Cvt::Tss2si); _cvt(T_1, Src0RM, Traits::Insts::Cvt::Tss2si);
_mov(T_2, T_1); // T_1 and T_2 may have different integer types _mov(T_2, T_1); // T_1 and T_2 may have different integer types
...@@ -2227,7 +2225,6 @@ void TargetX86Base<Machine>::lowerCast(const InstCast *Inst) { ...@@ -2227,7 +2225,6 @@ void TargetX86Base<Machine>::lowerCast(const InstCast *Inst) {
assert(Src0RM->getType() != IceType_i64); assert(Src0RM->getType() != IceType_i64);
T_1 = makeReg(IceType_i32); T_1 = makeReg(IceType_i32);
} }
T_1->setMustHaveReg();
Variable *T_2 = makeReg(Dest->getType()); Variable *T_2 = makeReg(Dest->getType());
if (Src0RM->getType() == T_1->getType()) if (Src0RM->getType() == T_1->getType())
_mov(T_1, Src0RM); _mov(T_1, Src0RM);
...@@ -2276,7 +2273,6 @@ void TargetX86Base<Machine>::lowerCast(const InstCast *Inst) { ...@@ -2276,7 +2273,6 @@ void TargetX86Base<Machine>::lowerCast(const InstCast *Inst) {
assert(Traits::Is64Bit || Src0RM->getType() != IceType_i32); assert(Traits::Is64Bit || Src0RM->getType() != IceType_i32);
T_1 = makeReg(IceType_i32); T_1 = makeReg(IceType_i32);
} }
T_1->setMustHaveReg();
Variable *T_2 = makeReg(Dest->getType()); Variable *T_2 = makeReg(Dest->getType());
if (Src0RM->getType() == T_1->getType()) if (Src0RM->getType() == T_1->getType())
_mov(T_1, Src0RM); _mov(T_1, Src0RM);
...@@ -2385,7 +2381,6 @@ void TargetX86Base<Machine>::lowerCast(const InstCast *Inst) { ...@@ -2385,7 +2381,6 @@ void TargetX86Base<Machine>::lowerCast(const InstCast *Inst) {
Variable *T = makeReg(IceType_f64); Variable *T = makeReg(IceType_f64);
// Movd requires its fp argument (in this case, the bitcast // Movd requires its fp argument (in this case, the bitcast
// destination) to be an xmm register. // destination) to be an xmm register.
T->setMustHaveReg();
_movd(T, Src0RM); _movd(T, Src0RM);
_mov(Dest, T); _mov(Dest, T);
} else { } else {
...@@ -2632,7 +2627,8 @@ void TargetX86Base<Machine>::lowerFcmp(const InstFcmp *Inst) { ...@@ -2632,7 +2627,8 @@ void TargetX86Base<Machine>::lowerFcmp(const InstFcmp *Inst) {
return; return;
} }
} }
Constant *Default = Ctx->getConstantInt32(Traits::TableFcmp[Index].Default); Constant *Default =
Ctx->getConstantInt(Dest->getType(), Traits::TableFcmp[Index].Default);
_mov(Dest, Default); _mov(Dest, Default);
if (HasC1) { if (HasC1) {
typename Traits::Insts::Label *Label = typename Traits::Insts::Label *Label =
...@@ -2642,7 +2638,7 @@ void TargetX86Base<Machine>::lowerFcmp(const InstFcmp *Inst) { ...@@ -2642,7 +2638,7 @@ void TargetX86Base<Machine>::lowerFcmp(const InstFcmp *Inst) {
_br(Traits::TableFcmp[Index].C2, Label); _br(Traits::TableFcmp[Index].C2, Label);
} }
Constant *NonDefault = Constant *NonDefault =
Ctx->getConstantInt32(!Traits::TableFcmp[Index].Default); Ctx->getConstantInt(Dest->getType(), !Traits::TableFcmp[Index].Default);
_mov_redefined(Dest, NonDefault); _mov_redefined(Dest, NonDefault);
Context.insert(Label); Context.insert(Label);
} }
...@@ -2819,8 +2815,8 @@ TargetX86Base<Machine>::lowerIcmp64(const InstIcmp *Inst) { ...@@ -2819,8 +2815,8 @@ TargetX86Base<Machine>::lowerIcmp64(const InstIcmp *Inst) {
// which needs the upper and lower halves legalized. // which needs the upper and lower halves legalized.
case InstIcmp::Sgt: case InstIcmp::Sgt:
case InstIcmp::Sle: case InstIcmp::Sle:
// These four compare after performing an "or" of the high and low half, so they // These four compare after performing an "or" of the high and low half, so
// need the upper and lower halves legalized. // they need the upper and lower halves legalized.
case InstIcmp::Eq: case InstIcmp::Eq:
case InstIcmp::Ule: case InstIcmp::Ule:
case InstIcmp::Ne: case InstIcmp::Ne:
...@@ -5186,7 +5182,6 @@ Operand *TargetX86Base<Machine>::legalize(Operand *From, LegalMask Allowed, ...@@ -5186,7 +5182,6 @@ Operand *TargetX86Base<Machine>::legalize(Operand *From, LegalMask Allowed,
if (Traits::Is64Bit) { if (Traits::Is64Bit) {
if (llvm::isa<ConstantInteger64>(Const)) { if (llvm::isa<ConstantInteger64>(Const)) {
Variable *V = copyToReg(Const, RegNum); Variable *V = copyToReg(Const, RegNum);
V->setMustHaveReg();
return V; return V;
} }
} }
......
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