Subzero. Refactors Switch Lowering.
BUG=
R=stichnot@chromium.org
Review URL: https://codereview.chromium.org/1860473002 .
diff --git a/src/IceCfg.cpp b/src/IceCfg.cpp
index e2bee72..33a5992 100644
--- a/src/IceCfg.cpp
+++ b/src/IceCfg.cpp
@@ -1025,11 +1025,6 @@
Str << Name << ":\n";
}
-void Cfg::deleteJumpTableInsts() {
- for (InstJumpTable *JumpTable : JumpTables)
- JumpTable->setDeleted();
-}
-
void Cfg::emitJumpTables() {
switch (getFlags().getOutFileType()) {
case FT_Elf:
@@ -1037,14 +1032,7 @@
// The emission needs to be delayed until the after the text section so
// save the offsets in the global context.
for (const InstJumpTable *JumpTable : JumpTables) {
- SizeT NumTargets = JumpTable->getNumTargets();
- JumpTableData::TargetList TargetList;
- for (SizeT I = 0; I < NumTargets; ++I) {
- SizeT Index = JumpTable->getTarget(I)->getIndex();
- TargetList.emplace_back(
- getAssembler()->getCfgNodeLabel(Index)->getPosition());
- }
- Ctx->addJumpTable(FunctionName, JumpTable->getId(), TargetList);
+ Ctx->addJumpTableData(JumpTable->toJumpTableData(getAssembler()));
}
} break;
case FT_Asm: {
@@ -1071,7 +1059,6 @@
const bool NeedSandboxing = getFlags().getUseSandboxing();
emitTextHeader(FunctionName, Ctx, Asm);
- deleteJumpTableInsts();
if (getFlags().getDecorateAsm()) {
for (Variable *Var : getVariables()) {
if (Var->getStackOffset() && !Var->isRematerializable()) {
@@ -1095,7 +1082,6 @@
TimerMarker T(TimerStack::TT_emitAsm, this);
// The emitIAS() routines emit into the internal assembler buffer, so there's
// no need to lock the streams.
- deleteJumpTableInsts();
const bool NeedSandboxing = getFlags().getUseSandboxing();
for (CfgNode *Node : Nodes) {
if (NeedSandboxing && Node->needsAlignment())
diff --git a/src/IceCfg.h b/src/IceCfg.h
index abb6722..a34a9ee 100644
--- a/src/IceCfg.h
+++ b/src/IceCfg.h
@@ -282,9 +282,6 @@
createBlockProfilingInfoDeclaration(const std::string &NodeAsmName,
VariableDeclaration *NodeNameDeclaration);
- /// Delete registered jump table placeholder instructions. This should only be
- /// called once all repointing has taken place.
- void deleteJumpTableInsts();
/// Iterate through the registered jump tables and emit them.
void emitJumpTables();
diff --git a/src/IceELFObjectWriter.cpp b/src/IceELFObjectWriter.cpp
index e960d9c..38919cc 100644
--- a/src/IceELFObjectWriter.cpp
+++ b/src/IceELFObjectWriter.cpp
@@ -581,12 +581,8 @@
const Elf64_Xword PointerSize = typeWidthInBytes(getPointerType());
const Elf64_Xword ShAddralign = PointerSize;
const Elf64_Xword ShEntsize = PointerSize;
- const GlobalString JTName = JT.getFunctionName();
const std::string SectionName = MangleSectionName(
- IsPIC ? ".data.rel.ro" : ".rodata",
- (JTName.hasStdString() ? JTName.toString()
- : std::to_string(JTName.getID())) +
- "$jumptable");
+ IsPIC ? ".data.rel.ro" : ".rodata", JT.getSectionName());
Section = createSection<ELFDataSection>(SectionName, SHT_PROGBITS, SHF_ALLOC,
ShAddralign, ShEntsize);
Section->setFileOffset(alignFileOffset(ShAddralign));
@@ -598,8 +594,7 @@
Section->padToAlignment(Str, PointerSize);
const bool IsExternal = getFlags().getDisableInternal();
const uint8_t SymbolBinding = IsExternal ? STB_GLOBAL : STB_LOCAL;
- GlobalString JumpTableName = Ctx.getGlobalString(
- InstJumpTable::makeName(JT.getFunctionName(), JT.getId()));
+ const auto JumpTableName = JT.getName();
SymTab->createDefinedSym(JumpTableName, SymbolType, SymbolBinding, Section,
Section->getCurrentSize(), PointerSize);
StrTab->add(JumpTableName);
diff --git a/src/IceGlobalContext.cpp b/src/IceGlobalContext.cpp
index e7faa50..25e1cc6 100644
--- a/src/IceGlobalContext.cpp
+++ b/src/IceGlobalContext.cpp
@@ -872,12 +872,8 @@
return JumpTables;
}
-JumpTableData &
-GlobalContext::addJumpTable(GlobalString FuncName, SizeT Id,
- const JumpTableData::TargetList &TargetList) {
- auto JumpTableList = getJumpTableList();
- JumpTableList->emplace_back(FuncName, Id, TargetList);
- return JumpTableList->back();
+void GlobalContext::addJumpTableData(JumpTableData JumpTable) {
+ getJumpTableList()->emplace_back(std::move(JumpTable));
}
TimerStackIdT GlobalContext::newTimerStackID(const std::string &Name) {
diff --git a/src/IceGlobalContext.h b/src/IceGlobalContext.h
index 2ce1083..7aaf513 100644
--- a/src/IceGlobalContext.h
+++ b/src/IceGlobalContext.h
@@ -268,9 +268,8 @@
/// Return a locked pointer to the registered jump tables.
JumpTableDataList getJumpTables();
- /// Create a new jump table entry and return a reference to it.
- JumpTableData &addJumpTable(GlobalString FuncName, SizeT Id,
- const JumpTableData::TargetList &TargetList);
+ /// Adds JumpTable to the list of know jump tables, for a posteriori emission.
+ void addJumpTableData(JumpTableData JumpTable);
/// Allocate data of type T using the global allocator. We allow entities
/// allocated from this global allocator to be either trivially or
diff --git a/src/IceInst.cpp b/src/IceInst.cpp
index 719791a..6cd2a04 100644
--- a/src/IceInst.cpp
+++ b/src/IceInst.cpp
@@ -569,12 +569,26 @@
InstFakeKill::InstFakeKill(Cfg *Func, const Inst *Linked)
: InstHighLevel(Func, Inst::FakeKill, 0, nullptr), Linked(Linked) {}
+namespace {
+GlobalString makeName(Cfg *Func, const SizeT Id) {
+ const auto FuncName = Func->getFunctionName();
+ auto *Ctx = Func->getContext();
+ if (FuncName.hasStdString())
+ return GlobalString::createWithString(
+ Ctx, ".L" + FuncName.toString() + "$jumptable$__" + std::to_string(Id));
+ return GlobalString::createWithString(
+ Ctx, ".L" + std::to_string(FuncName.getID()) + "_" + std::to_string(Id));
+}
+} // end of anonymous namespace
+
InstJumpTable::InstJumpTable(Cfg *Func, SizeT NumTargets, CfgNode *Default)
: InstHighLevel(Func, Inst::JumpTable, 1, nullptr),
- Id(Func->getTarget()->makeNextJumpTableNumber()), NumTargets(NumTargets) {
+ Id(Func->getTarget()->makeNextJumpTableNumber()), NumTargets(NumTargets),
+ Name(makeName(Func, Id)), FuncName(Func->getFunctionName()) {
Targets = Func->allocateArrayOf<CfgNode *>(NumTargets);
- for (SizeT I = 0; I < NumTargets; ++I)
+ for (SizeT I = 0; I < NumTargets; ++I) {
Targets[I] = Default;
+ }
}
bool InstJumpTable::repointEdges(CfgNode *OldNode, CfgNode *NewNode) {
@@ -588,6 +602,15 @@
return Found;
}
+JumpTableData InstJumpTable::toJumpTableData(Assembler *Asm) const {
+ JumpTableData::TargetList TargetList(NumTargets);
+ for (SizeT i = 0; i < NumTargets; ++i) {
+ const SizeT Index = Targets[i]->getIndex();
+ TargetList[i] = Asm->getCfgNodeLabel(Index)->getPosition();
+ }
+ return JumpTableData(Name, FuncName, Id, TargetList);
+}
+
Type InstCall::getReturnType() const {
if (Dest == nullptr)
return IceType_void;
diff --git a/src/IceInst.h b/src/IceInst.h
index dfb873c..1b49419 100644
--- a/src/IceInst.h
+++ b/src/IceInst.h
@@ -22,6 +22,7 @@
#include "IceDefs.h"
#include "IceInst.def"
#include "IceIntrinsics.h"
+#include "IceSwitchLowering.h"
#include "IceTypes.h"
// TODO: The Cfg structure, and instructions in particular, need to be
@@ -943,12 +944,22 @@
static bool classof(const Inst *Instr) {
return Instr->getKind() == JumpTable;
}
+ // Creates a JumpTableData struct (used for ELF emission) that represents this
+ // InstJumpTable.
+ JumpTableData toJumpTableData(Assembler *Asm) const;
- // TODO(stichnot): Should this create&save GlobalString values?
- static std::string makeName(GlobalString FuncName, SizeT Id) {
- if (FuncName.hasStdString())
- return ".L" + FuncName + "$jumptable$__" + std::to_string(Id);
- return ".L" + std::to_string(FuncName.getID()) + "_" + std::to_string(Id);
+ // InstJumpTable is just a placeholder for the switch targets, and it does not
+ // need to emit any code, so we redefine emit and emitIAS to do nothing.
+ void emit(const Cfg *) const override {}
+ void emitIAS(const Cfg * /* Func */) const override {}
+
+ const std::string getName() const {
+ assert(Name.hasStdString());
+ return Name.toString();
+ }
+
+ std::string getSectionName() const {
+ return JumpTableData::createSectionName(FuncName);
}
private:
@@ -961,6 +972,8 @@
const SizeT Id;
const SizeT NumTargets;
CfgNode **Targets;
+ GlobalString Name; // This JumpTable's name in the output.
+ GlobalString FuncName;
};
/// The Target instruction is the base class for all target-specific
diff --git a/src/IceSwitchLowering.cpp b/src/IceSwitchLowering.cpp
index 35749bf..ede1a94 100644
--- a/src/IceSwitchLowering.cpp
+++ b/src/IceSwitchLowering.cpp
@@ -23,10 +23,11 @@
CaseClusterArray CaseCluster::clusterizeSwitch(Cfg *Func,
const InstSwitch *Instr) {
+ const SizeT NumCases = Instr->getNumCases();
CaseClusterArray CaseClusters;
+ CaseClusters.reserve(NumCases);
// Load the cases
- SizeT NumCases = Instr->getNumCases();
CaseClusters.reserve(NumCases);
for (SizeT I = 0; I < NumCases; ++I)
CaseClusters.emplace_back(Instr->getValue(I), Instr->getLabel(I));
@@ -60,21 +61,21 @@
// frequently executed code but we can't do this well without profiling data.
// So, this single jump table is a good starting point where you can get to
// the jump table quickly without figuring out how to unbalance the tree.
- uint64_t MaxValue = CaseClusters.back().High;
- uint64_t MinValue = CaseClusters.front().Low;
+ const uint64_t MaxValue = CaseClusters.back().High;
+ const uint64_t MinValue = CaseClusters.front().Low;
// Don't +1 yet to avoid (INT64_MAX-0)+1 overflow
- uint64_t TotalRange = MaxValue - MinValue;
+ const uint64_t Range = MaxValue - MinValue;
// Might be too sparse for the jump table
- if (NumCases * 2 <= TotalRange)
+ if (NumCases * 2 <= Range)
return CaseClusters;
// Unlikely. Would mean can't store size of jump table.
- if (TotalRange == UINT64_MAX)
+ if (Range == UINT64_MAX)
return CaseClusters;
- ++TotalRange;
+ const uint64_t TotalRange = Range + 1;
// Replace everything with a jump table
- InstJumpTable *JumpTable =
+ auto *JumpTable =
InstJumpTable::create(Func, TotalRange, Instr->getLabelDefault());
for (const CaseCluster &Case : CaseClusters) {
// Case.High could be UINT64_MAX which makes the loop awkward. Unwrap the
@@ -94,7 +95,8 @@
bool CaseCluster::tryAppend(const CaseCluster &New) {
// Can only append ranges with the same target and are adjacent
- bool CanAppend = this->Target == New.Target && this->High + 1 == New.Low;
+ const bool CanAppend =
+ this->Target == New.Target && this->High + 1 == New.Low;
if (CanAppend)
this->High = New.High;
return CanAppend;
diff --git a/src/IceSwitchLowering.h b/src/IceSwitchLowering.h
index d4f5291..57dd828 100644
--- a/src/IceSwitchLowering.h
+++ b/src/IceSwitchLowering.h
@@ -18,6 +18,8 @@
#include "IceDefs.h"
#include "IceStringPool.h"
+#include <string>
+
namespace Ice {
class CaseCluster;
@@ -85,18 +87,27 @@
public:
using TargetList = std::vector<intptr_t>;
- JumpTableData(GlobalString FuncName, SizeT Id,
+ JumpTableData(GlobalString Name, GlobalString FuncName, SizeT Id,
const TargetList &TargetOffsets)
- : FuncName(FuncName), Id(Id), TargetOffsets(TargetOffsets) {}
+ : Name(Name), FuncName(FuncName), Id(Id), TargetOffsets(TargetOffsets) {}
JumpTableData(const JumpTableData &) = default;
JumpTableData(JumpTableData &&) = default;
JumpTableData &operator=(JumpTableData &&) = default;
- const GlobalString getFunctionName() const { return FuncName; }
+ GlobalString getName() const { return Name; }
+ GlobalString getFunctionName() const { return FuncName; }
SizeT getId() const { return Id; }
const TargetList &getTargetOffsets() const { return TargetOffsets; }
+ static std::string createSectionName(const GlobalString Name) {
+ if (Name.hasStdString()) {
+ return Name.toString() + "$jumptable";
+ }
+ return std::to_string(Name.getID()) + "$jumptable";
+ }
+ std::string getSectionName() const { return createSectionName(FuncName); }
private:
+ GlobalString Name;
GlobalString FuncName;
SizeT Id;
TargetList TargetOffsets;
diff --git a/src/IceTargetLoweringX86BaseImpl.h b/src/IceTargetLoweringX86BaseImpl.h
index a344d57..c1a5f74 100644
--- a/src/IceTargetLoweringX86BaseImpl.h
+++ b/src/IceTargetLoweringX86BaseImpl.h
@@ -5881,10 +5881,8 @@
}
constexpr RelocOffsetT RelocOffset = 0;
- GlobalString FunctionName = Func->getFunctionName();
constexpr Variable *NoBase = nullptr;
- auto JTName = GlobalString::createWithString(
- Ctx, InstJumpTable::makeName(FunctionName, JumpTable->getId()));
+ auto JTName = GlobalString::createWithString(Ctx, JumpTable->getName());
Constant *Offset = Ctx->getConstantSym(RelocOffset, JTName);
uint16_t Shift = typeWidthInBytesLog2(PointerType);
constexpr auto Segment = X86OperandMem::SegmentRegisters::DefaultSegment;
@@ -7277,17 +7275,16 @@
template <typename TraitsType>
void TargetX86Base<TraitsType>::emitJumpTable(
- const Cfg *Func, const InstJumpTable *JumpTable) const {
+ const Cfg *, const InstJumpTable *JumpTable) const {
if (!BuildDefs::dump())
return;
Ostream &Str = Ctx->getStrEmit();
const bool UseNonsfi = getFlags().getUseNonsfi();
- GlobalString FunctionName = Func->getFunctionName();
const char *Prefix = UseNonsfi ? ".data.rel.ro." : ".rodata.";
- Str << "\t.section\t" << Prefix << FunctionName
- << "$jumptable,\"a\",@progbits\n";
- Str << "\t.align\t" << typeWidthInBytes(getPointerType()) << "\n";
- Str << InstJumpTable::makeName(FunctionName, JumpTable->getId()) << ":";
+ Str << "\t.section\t" << Prefix << JumpTable->getSectionName()
+ << ",\"a\",@progbits\n"
+ "\t.align\t" << typeWidthInBytes(getPointerType()) << "\n"
+ << JumpTable->getName() << ":";
// On X86 ILP32 pointers are 32-bit hence the use of .long
for (SizeT I = 0; I < JumpTable->getNumTargets(); ++I)
@@ -7389,10 +7386,10 @@
Ostream &Str = Ctx->getStrEmit();
const char *Prefix = IsPIC ? ".data.rel.ro." : ".rodata.";
for (const JumpTableData &JT : Ctx->getJumpTables()) {
- Str << "\t.section\t" << Prefix << JT.getFunctionName()
- << "$jumptable,\"a\",@progbits\n";
- Str << "\t.align\t" << typeWidthInBytes(getPointerType()) << "\n";
- Str << InstJumpTable::makeName(JT.getFunctionName(), JT.getId()) << ":";
+ Str << "\t.section\t" << Prefix << JT.getSectionName()
+ << ",\"a\",@progbits\n"
+ "\t.align\t" << typeWidthInBytes(getPointerType()) << "\n"
+ << JT.getName().toString() << ":";
// On X8664 ILP32 pointers are 32-bit hence the use of .long
for (intptr_t TargetOffset : JT.getTargetOffsets())
