Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(34)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: src/IceInstARM32.cpp

Issue 1535233002: Refactor PUSH/POP in ARM assemblers. (Closed) Base URL: https://chromium.googlesource.com/native_client/pnacl-subzero.git@master
Patch Set: Refactor and fix trivial bug (insertion of newline for vpush). Created 4 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« src/IceInstARM32.h ('K') | « src/IceInstARM32.h ('k') | src/IceRegistersARM32.h » ('j') | src/IceRegistersARM32.h » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/IceInstARM32.cpp
diff --git a/src/IceInstARM32.cpp b/src/IceInstARM32.cpp
index 4919600ddb228355fd2b09f7deca52563fa18c8e..3539c093662de9187224762a0f191e7384c466dd 100644
--- a/src/IceInstARM32.cpp
+++ b/src/IceInstARM32.cpp
@@ -88,6 +88,19 @@ CondARM32::Cond InstARM32::getOppositeCondition(CondARM32::Cond Cond) {
return InstARM32CondAttributes[Cond].Opposite;
}
+template <>
+void InstARM32::startNextAsmInst<InstARM32::TextualOutput>(
+ const Cfg *Func) const {
+ startNextInst(Func);
+ Func->getContext()->getStrEmit() << "\n";
+}
+
+template <>
+void InstARM32::startNextAsmInst<InstARM32::BinaryOutput>(
John 2016/01/06 16:10:21 The fact that this does nothing tells me this meth
Karl 2016/01/06 23:21:58 Removed.
+ const Cfg *Func) const {
+ (void)Func;
+}
+
void InstARM32::startNextInst(const Cfg *Func) const {
if (auto *Asm = Func->getAssembler<ARM32::AssemblerARM32>())
Asm->incEmitTextSize(InstSize);
@@ -1324,67 +1337,84 @@ bool isAssignedConsecutiveRegisters(const Variable *Before,
} // end of anonymous namespace
-void InstARM32Pop::emit(const Cfg *Func) const {
+void InstARM32Pop::emitPop(const Cfg *Func) const {
if (!BuildDefs::dump())
return;
-
+ Ostream &Str = Func->getContext()->getStrEmit();
+ Str << "\t"
+ << "pop"
+ << "\t{";
+ Dests[0]->emit(Func);
const SizeT DestSize = Dests.size();
- if (DestSize == 0) {
- assert(false && "Empty pop list");
- return;
+ for (SizeT i = 1; i < DestSize; ++i) {
+ Str << ", ";
+ Dests[i]->emit(Func);
}
+ Str << "}";
+}
- Ostream &Str = Func->getContext()->getStrEmit();
+template <>
+void InstARM32Pop::emitPop<InstARM32::TextualOutput>(
+ const Cfg *Func, const Variable *Reg) const {
+ (void)Reg;
+ this->emitPop(Func);
+}
- Variable *Reg = Dests[0];
- if (isScalarIntegerType(Reg->getType())) {
- // GPR push.
- Str << "\t"
- "pop"
- "\t{";
- Reg->emit(Func);
- for (SizeT i = 1; i < DestSize; ++i) {
- Str << ", ";
- Reg = Dests[i];
- Reg->emit(Func);
- }
- Str << "}";
- return;
- }
+template <>
+void InstARM32Pop::emitPop<InstARM32::BinaryOutput>(const Cfg *Func,
+ const Variable *Reg) const {
+ auto *Asm = Func->getAssembler<ARM32::AssemblerARM32>();
+ Asm->pop(Reg, CondARM32::AL);
+}
+
+template <>
+void InstARM32Pop::emitPopList<InstARM32::TextualOutput>(
+ const Cfg *Func, ARM32::IValueT Registers) const {
+ (void)Registers;
+ emitPop(Func);
+}
+
+template <>
+void InstARM32Pop::emitPopList<InstARM32::BinaryOutput>(
+ const Cfg *Func, ARM32::IValueT Registers) const {
+ auto *Asm = Func->getAssembler<ARM32::AssemblerARM32>();
+ Asm->popList(Registers, CondARM32::AL);
+}
- // VFP "s" reg push.
- SizeT End = DestSize - 1;
- SizeT Start = DestSize - 1;
- Reg = Dests[DestSize - 1];
+template <>
+void InstARM32Pop::emitVpop<InstARM32::TextualOutput>(const Cfg *Func,
+ const Variable *BaseReg,
+ SizeT RegCount) const {
+ if (!BuildDefs::dump())
+ return;
+ Ostream &Str = Func->getContext()->getStrEmit();
Str << "\t"
- "vpop"
- "\t{";
- for (SizeT i = 2; i <= DestSize; ++i) {
- Variable *PreviousReg = Dests[DestSize - i];
- if (!isAssignedConsecutiveRegisters(PreviousReg, Reg)) {
- Dests[Start]->emit(Func);
- for (SizeT j = Start + 1; j <= End; ++j) {
- Str << ", ";
- Dests[j]->emit(Func);
- }
- startNextInst(Func);
- Str << "}\n\t"
- "vpop"
- "\t{";
- End = DestSize - i;
- }
- Reg = PreviousReg;
- Start = DestSize - i;
- }
- Dests[Start]->emit(Func);
- for (SizeT j = Start + 1; j <= End; ++j) {
- Str << ", ";
- Dests[j]->emit(Func);
+ << "vpop"
+ << "\t{";
+ bool IsFirst = true;
+ IValueT Base = AssemblerARM32::getEncodedSRegNum(BaseReg);
John 2016/01/06 16:10:21 Why using this method? RegARM32::getEncodedSReg()
Karl 2016/01/06 23:21:58 Fixed.
+ for (SizeT i = 0; i < RegCount; ++i) {
+ if (IsFirst)
+ IsFirst = false;
+ else
+ Str << ", ";
+ Str << RegARM32::getSRegName(Base + i);
}
Str << "}";
}
-void InstARM32Pop::emitIAS(const Cfg *Func) const {
+template <>
+void InstARM32Pop::emitVpop<InstARM32::BinaryOutput>(const Cfg *Func,
+ const Variable *BaseReg,
+ SizeT RegCount) const {
+ auto *Asm = Func->getAssembler<ARM32::AssemblerARM32>();
+ Asm->vpop(BaseReg, RegCount, CondARM32::AL);
+}
+
+template <InstARM32::OutputForm Form>
+void InstARM32Pop::assemble(const Cfg *Func) const {
+ if (!BuildDefs::dump() && Form == TextualOutput)
+ return;
// Pop can't be emitted if there are no registers to load. This should never
// happen, but if it does, we don't need to bring Subzero down -- we just skip
// emitting the pop instruction (and maybe emit a nop?) The assert() is here
@@ -1395,7 +1425,6 @@ void InstARM32Pop::emitIAS(const Cfg *Func) const {
return;
}
- auto *Asm = Func->getAssembler<ARM32::AssemblerARM32>();
const auto *Reg = llvm::cast<Variable>(Dests[0]);
if (isScalarIntegerType(Reg->getType())) {
// Pop GPR registers.
@@ -1416,10 +1445,10 @@ void InstARM32Pop::emitIAS(const Cfg *Func) const {
// Note: Can only apply pop register if single register is not sp.
assert((RegARM32::Encoded_Reg_sp != LastDest->getRegNum()) &&
"Effects of pop register SP is undefined!");
- Asm->pop(LastDest, CondARM32::AL);
+ emitPop<Form>(Func, LastDest);
break;
default:
- Asm->popList(GPRegisters, CondARM32::AL);
+ emitPopList<Form>(Func, GPRegisters);
break;
}
} else {
@@ -1434,17 +1463,18 @@ void InstARM32Pop::emitIAS(const Cfg *Func) const {
isAssignedConsecutiveRegisters(Reg, NextReg)) {
++RegCount;
} else {
- Asm->vpop(BaseReg, RegCount, CondARM32::AL);
+ emitVpop<Form>(Func, BaseReg, RegCount);
+ this->startNextAsmInst<Form>(Func);
BaseReg = NextReg;
RegCount = 1;
}
Reg = NextReg;
}
- if (RegCount)
- Asm->vpop(BaseReg, RegCount, CondARM32::AL);
+ if (RegCount) {
+ emitVpop<Form>(Func, BaseReg, RegCount);
+ }
}
- if (Asm->needsTextFixup())
- emitUsingTextFixup(Func);
+ assert(!Func->getAssembler<ARM32::AssemblerARM32>()->needsTextFixup());
}
void InstARM32Pop::dump(const Cfg *Func) const {
@@ -1460,63 +1490,92 @@ void InstARM32Pop::dump(const Cfg *Func) const {
}
}
-void InstARM32Push::emit(const Cfg *Func) const {
+void InstARM32Pop::emit(const Cfg *Func) const {
+ assemble<TextualOutput>(Func);
John 2016/01/06 16:10:21 I would rather have the if (!BuildDefs::dump()) {
Karl 2016/01/06 23:21:58 Done.
+}
+
+void InstARM32Pop::emitIAS(const Cfg *Func) const {
+ assemble<BinaryOutput>(Func);
+}
+
+void InstARM32Push::emitPush(const Cfg *Func) const {
if (!BuildDefs::dump())
return;
-
- // Push can't be emitted if there are no registers to save. This should never
- // happen, but if it does, we don't need to bring Subzero down -- we just skip
- // emitting the push instruction (and maybe emit a nop?) The assert() is here
- // so that we can detect this error during development.
+ Ostream &Str = Func->getContext()->getStrEmit();
+ Str << "\t"
+ << "push"
+ << "\t{";
+ getSrc(0)->emit(Func);
const SizeT SrcSize = getSrcSize();
- if (SrcSize == 0) {
- assert(false && "Empty push list");
- return;
+ for (SizeT i = 1; i < SrcSize; ++i) {
+ Str << ", ";
+ getSrc(i)->emit(Func);
}
+ Str << "}";
+}
- Ostream &Str = Func->getContext()->getStrEmit();
+template <>
+void InstARM32Push::emitPush<InstARM32::TextualOutput>(
+ const Cfg *Func, const Variable *Reg) const {
+ (void)Reg;
+ this->emitPush(Func);
John 2016/01/06 16:10:21 do you need the this-> ?
Karl 2016/01/06 23:21:58 Removed.
+}
- const auto *Reg = llvm::cast<Variable>(getSrc(0));
- if (isScalarIntegerType(Reg->getType())) {
- // GPR push.
- Str << "\t"
- "push"
- "\t{";
- Reg->emit(Func);
- for (SizeT i = 1; i < SrcSize; ++i) {
- Str << ", ";
- getSrc(i)->emit(Func);
- }
- Str << "}";
- return;
- }
+template <>
+void InstARM32Push::emitPush<InstARM32::BinaryOutput>(
+ const Cfg *Func, const Variable *Reg) const {
+ auto *Asm = Func->getAssembler<ARM32::AssemblerARM32>();
+ Asm->push(Reg, CondARM32::AL);
+}
- // VFP "s" reg push.
+template <>
+void InstARM32Push::emitPushList<InstARM32::TextualOutput>(
+ const Cfg *Func, ARM32::IValueT Registers) const {
+ (void)Registers;
+ emitPush(Func);
+}
+
+template <>
+void InstARM32Push::emitPushList<InstARM32::BinaryOutput>(
+ const Cfg *Func, ARM32::IValueT Registers) const {
+ auto *Asm = Func->getAssembler<ARM32::AssemblerARM32>();
+ Asm->pushList(Registers, CondARM32::AL);
+}
+
+template <>
+void InstARM32Push::emitVPush<InstARM32::TextualOutput>(const Cfg *Func,
+ const Variable *BaseReg,
+ SizeT RegCount) const {
+ if (!BuildDefs::dump())
+ return;
+ Ostream &Str = Func->getContext()->getStrEmit();
Str << "\t"
- "vpush"
- "\t{";
- Reg->emit(Func);
- SizeT RegCount = 1;
- for (SizeT i = 1; i < SrcSize; ++i) {
- const auto *NextReg = llvm::cast<Variable>(getSrc(i));
- if (RegCount < VpushVpopMaxConsecRegs &&
- isAssignedConsecutiveRegisters(Reg, NextReg)) {
- ++RegCount;
+ << "vpush"
+ << "\t{";
+ bool IsFirst = true;
+ IValueT Base = AssemblerARM32::getEncodedSRegNum(BaseReg);
John 2016/01/06 16:10:21 ditto.
Karl 2016/01/06 23:21:58 Done.
+ for (SizeT i = 0; i < RegCount; ++i) {
+ if (IsFirst)
+ IsFirst = false;
+ else
Str << ", ";
- } else {
- startNextInst(Func);
- RegCount = 1;
- Str << "}\n\t"
- "vpush"
- "\t{";
- }
- Reg = NextReg;
- Reg->emit(Func);
+ Str << RegARM32::getSRegName(Base + i);
}
Str << "}";
}
-void InstARM32Push::emitIAS(const Cfg *Func) const {
+template <>
+void InstARM32Push::emitVPush<InstARM32::BinaryOutput>(const Cfg *Func,
+ const Variable *BaseReg,
+ SizeT RegCount) const {
+ auto *Asm = Func->getAssembler<ARM32::AssemblerARM32>();
+ Asm->vpush(BaseReg, RegCount, CondARM32::AL);
+}
+
+template <InstARM32::OutputForm Form>
+void InstARM32Push::assemble(const Cfg *Func) const {
John 2016/01/06 16:10:21 assemble is not really meaningful for textual outp
Karl 2016/01/06 23:21:58 renamed to emitUsingForm().
+ if (!BuildDefs::dump() && Form == TextualOutput)
+ return;
// Push can't be emitted if there are no registers to save. This should never
// happen, but if it does, we don't need to bring Subzero down -- we just skip
// emitting the push instruction (and maybe emit a nop?) The assert() is here
@@ -1527,7 +1586,6 @@ void InstARM32Push::emitIAS(const Cfg *Func) const {
return;
}
- auto *Asm = Func->getAssembler<ARM32::AssemblerARM32>();
const auto *Reg = llvm::cast<Variable>(getSrc(0));
if (isScalarIntegerType(Reg->getType())) {
// Push GPR registers.
@@ -1549,11 +1607,11 @@ void InstARM32Push::emitIAS(const Cfg *Func) const {
// Note: Can only apply push register if single register is not sp.
assert((RegARM32::Encoded_Reg_sp != LastSrc->getRegNum()) &&
"Effects of push register SP is undefined!");
- Asm->push(LastSrc, CondARM32::AL);
+ emitPush<Form>(Func, LastSrc);
break;
}
default:
- Asm->pushList(GPRegisters, CondARM32::AL);
+ emitPushList<Form>(Func, GPRegisters);
break;
}
} else {
@@ -1566,16 +1624,24 @@ void InstARM32Push::emitIAS(const Cfg *Func) const {
isAssignedConsecutiveRegisters(Reg, NextReg)) {
++RegCount;
} else {
- Asm->vpush(BaseReg, RegCount, CondARM32::AL);
+ emitVPush<Form>(Func, BaseReg, RegCount);
+ this->startNextAsmInst<Form>(Func);
BaseReg = NextReg;
RegCount = 1;
}
Reg = NextReg;
}
- Asm->vpush(BaseReg, RegCount, CondARM32::AL);
+ emitVPush<Form>(Func, BaseReg, RegCount);
}
- if (Asm->needsTextFixup())
- emitUsingTextFixup(Func);
+ assert(!Func->getAssembler<ARM32::AssemblerARM32>()->needsTextFixup());
+}
+
+void InstARM32Push::emit(const Cfg *Func) const {
+ assemble<TextualOutput>(Func);
+}
+
+void InstARM32Push::emitIAS(const Cfg *Func) const {
+ assemble<BinaryOutput>(Func);
}
void InstARM32Push::dump(const Cfg *Func) const {
« src/IceInstARM32.h ('K') | « src/IceInstARM32.h ('k') | src/IceRegistersARM32.h » ('j') | src/IceRegistersARM32.h » ('J')

Powered by Google App Engine
This is Rietveld 408576698