Subzero. Refactors Switch Lowering.

src/IceCfg.cpp

@@ -1025,11 +1025,6 @@ void Cfg::emitTextHeader(GlobalString Name, GlobalContext *Ctx,
   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 @@ void Cfg::emitJumpTables() {
     // 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 @@ void Cfg::emit() {
   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 @@ void Cfg::emitIAS() {
   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())

src/IceCfg.h

@@ -282,9 +282,6 @@ private:
   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();
 

src/IceELFObjectWriter.cpp

@@ -581,12 +581,8 @@ void ELFObjectWriter::writeJumpTable(const JumpTableData &JT,
   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 @@ void ELFObjectWriter::writeJumpTable(const JumpTableData &JT,
   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);

src/IceGlobalContext.cpp

@@ -872,12 +872,8 @@ JumpTableDataList GlobalContext::getJumpTables() {
   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) {

src/IceGlobalContext.h

@@ -268,9 +268,8 @@ public:
 
   /// 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

src/IceInst.cpp

@@ -569,12 +569,26 @@ InstFakeUse::InstFakeUse(Cfg *Func, Variable *Src, uint32_t Weight)
 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 @@ bool InstJumpTable::repointEdges(CfgNode *OldNode, CfgNode *NewNode) {
   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;

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 @@ public:
   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 @@ private:
   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

src/IceSwitchLowering.cpp

@@ -23,10 +23,11 @@ namespace Ice {
 
 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 @@ CaseClusterArray CaseCluster::clusterizeSwitch(Cfg *Func,
   // 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 @@ CaseClusterArray CaseCluster::clusterizeSwitch(Cfg *Func,
 
 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;

src/IceSwitchLowering.h

@@ -18,6 +18,8 @@
 #include "IceDefs.h"
 #include "IceStringPool.h"
 
+#include <string>
+
 namespace Ice {
 
 class CaseCluster;
@@ -85,18 +87,27 @@ class JumpTableData {
 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;

src/IceTargetLoweringX86BaseImpl.h

@@ -5881,10 +5881,8 @@ void TargetX86Base<TraitsType>::lowerCaseCluster(const CaseCluster &Case,
     }
 
     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 @@ TargetX86Base<TraitsType>::randomizeOrPoolImmediate(X86OperandMem *MemOperand,
 
 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 @@ void TargetDataX86<TraitsType>::lowerJumpTables() {
     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())