Subzero: Don't store std::string objects inside Variable.

src/IceCfg.h

@@ -62,14 +62,24 @@ public:
   CfgNode *makeNode();
   SizeT getNumNodes() const { return Nodes.size(); }
   const NodeList &getNodes() const { return Nodes; }
+
+  typedef int32_t IdentifierIndexType;
   // Adds a name to the list and returns its index, suitable for the
-  // argument to getNodeName().  No checking for duplicates is done.
-  int32_t addNodeName(const IceString &Name) {
-    int32_t Index = NodeNames.size();
-    NodeNames.push_back(Name);
+  // argument to getIdentifierName().  No checking for duplicates is
+  // done.  This is generally used for node names and variable names
+  // to avoid embedding a std::string inside an arena-allocated
+  // object.
+  IdentifierIndexType addIdentifierName(const IceString &Name) {
+    IdentifierIndexType Index = IdentifierNames.size();
+    IdentifierNames.push_back(Name);
     return Index;
   }
-  const IceString &getNodeName(int32_t Index) const { return NodeNames[Index]; }
+  const IceString &getIdentifierName(IdentifierIndexType Index) const {
+    return IdentifierNames[Index];
+  }
+  enum {
+    IdentifierIndexInvalid = -1
+  };
 
   // Manage instruction numbering.
   InstNumberT newInstNumber() { return NextInstNumber++; }
@@ -78,10 +88,9 @@ public:
   // Manage Variables.
   // Create a new Variable with a particular type and an optional
   // name.  The Node argument is the node where the variable is defined.
-  template <typename T = Variable>
-  T *makeVariable(Type Ty, const IceString &Name = "") {
+  template <typename T = Variable> T *makeVariable(Type Ty) {
     SizeT Index = Variables.size();
-    T *Var = T::create(this, Ty, Index, Name);
+    T *Var = T::create(this, Ty, Index);
     Variables.push_back(Var);
     return Var;
   }
@@ -183,7 +192,7 @@ private:
   IceString ErrorMessage;
   CfgNode *Entry; // entry basic block
   NodeList Nodes; // linearized node list; Entry should be first
-  std::vector<IceString> NodeNames;
+  std::vector<IceString> IdentifierNames;
   InstNumberT NextInstNumber;
   VarList Variables;
   VarList Args; // subset of Variables, in argument order

src/IceCfgNode.cpp

@@ -23,14 +23,14 @@
 namespace Ice {
 
 CfgNode::CfgNode(Cfg *Func, SizeT LabelNumber)
-    : Func(Func), Number(LabelNumber), NameIndex(-1), HasReturn(false),
-      NeedsPlacement(false), InstCountEstimate(0) {}
+    : Func(Func), Number(LabelNumber), NameIndex(Cfg::IdentifierIndexInvalid),
+      HasReturn(false), NeedsPlacement(false), InstCountEstimate(0) {}
 
 // Returns the name the node was created with.  If no name was given,
 // it synthesizes a (hopefully) unique name.
 IceString CfgNode::getName() const {
   if (NameIndex >= 0)
-    return Func->getNodeName(NameIndex);
+    return Func->getIdentifierName(NameIndex);
   return "__" + std::to_string(getIndex());
 }
 
@@ -437,8 +437,9 @@ void CfgNode::advancedPhiLowering() {
           Operand *OtherSrc = Desc[J].Src;
           if (Desc[J].NumPred && sameVarOrReg(Dest, OtherSrc)) {
             SizeT VarNum = Func->getNumVariables();
-            Variable *Tmp = Func->makeVariable(
-                OtherSrc->getType(), "__split_" + std::to_string(VarNum));
+            Variable *Tmp = Func->makeVariable(OtherSrc->getType());
+            if (ALLOW_DUMP)
+              Tmp->setName(Func, "__split_" + std::to_string(VarNum));
             Assignments.push_back(InstAssign::create(Func, Tmp, OtherSrc));
             Desc[J].Src = Tmp;
             Found = true;
@@ -970,7 +971,7 @@ void CfgNode::dump(Cfg *Func) const {
     for (SizeT i = 0; i < LiveIn.size(); ++i) {
       if (LiveIn[i]) {
         Variable *Var = Liveness->getVariable(i, this);
-        Str << " %" << Var->getName();
+        Str << " %" << Var->getName(Func);
         if (Func->getContext()->isVerbose(IceV_RegOrigins) && Var->hasReg()) {
           Str << ":" << Func->getTarget()->getRegName(Var->getRegNum(),
                                                       Var->getType());
@@ -995,7 +996,7 @@ void CfgNode::dump(Cfg *Func) const {
     for (SizeT i = 0; i < LiveOut.size(); ++i) {
       if (LiveOut[i]) {
         Variable *Var = Liveness->getVariable(i, this);
-        Str << " %" << Var->getName();
+        Str << " %" << Var->getName(Func);
         if (Func->getContext()->isVerbose(IceV_RegOrigins) && Var->hasReg()) {
           Str << ":" << Func->getTarget()->getRegName(Var->getRegNum(),
                                                       Var->getType());

src/IceCfgNode.h

@@ -35,9 +35,9 @@ public:
   IceString getName() const;
   void setName(const IceString &NewName) {
     // Make sure that the name can only be set once.
-    assert(NameIndex < 0);
+    assert(NameIndex == Cfg::IdentifierIndexInvalid);
     if (!NewName.empty())
-      NameIndex = Func->addNodeName(NewName);
+      NameIndex = Func->addIdentifierName(NewName);
   }
   IceString getAsmName() const {
     return ".L" + Func->getFunctionName() + "$" + getName();
@@ -91,7 +91,7 @@ private:
   CfgNode(Cfg *Func, SizeT LabelIndex);
   Cfg *const Func;
   const SizeT Number; // label index
-  int32_t NameIndex;  // index into Cfg::NodeNames table
+  Cfg::IdentifierIndexType NameIndex; // index into Cfg::NodeNames table
   bool HasReturn;     // does this block need an epilog?
   bool NeedsPlacement;
   InstNumberT InstCountEstimate; // rough instruction count estimate

src/IceConverter.cpp

@@ -145,7 +145,9 @@ private:
     if (IceTy == Ice::IceType_void)
       return nullptr;
     if (VarMap.find(V) == VarMap.end()) {
-      VarMap[V] = Func->makeVariable(IceTy, V->getName());
+      VarMap[V] = Func->makeVariable(IceTy);
+      if (ALLOW_DUMP)
+        VarMap[V]->setName(Func, V->getName());
     }
     return VarMap[V];
   }

src/IceInst.cpp

@@ -361,8 +361,9 @@ void InstPhi::livenessPhiOperand(LivenessBV &Live, CfgNode *Target,
 Inst *InstPhi::lower(Cfg *Func) {
   Variable *Dest = getDest();
   assert(Dest);
-  IceString PhiName = Dest->getName() + "_phi";
-  Variable *NewSrc = Func->makeVariable(Dest->getType(), PhiName);
+  Variable *NewSrc = Func->makeVariable(Dest->getType());
+  if (ALLOW_DUMP)
+    NewSrc->setName(Func, Dest->getName(Func) + "_phi");
   this->Dest = NewSrc;
   return InstAssign::create(Func, Dest, NewSrc);
 }

src/IceInstX8632.h

@@ -152,9 +152,8 @@ class SpillVariable : public Variable {
   SpillVariable &operator=(const SpillVariable &) = delete;
 
 public:
-  static SpillVariable *create(Cfg *Func, Type Ty, SizeT Index,
-                               const IceString &Name) {
-    return new (Func->allocate<SpillVariable>()) SpillVariable(Ty, Index, Name);
+  static SpillVariable *create(Cfg *Func, Type Ty, SizeT Index) {
+    return new (Func->allocate<SpillVariable>()) SpillVariable(Ty, Index);
   }
   const static OperandKind SpillVariableKind =
       static_cast<OperandKind>(kVariable_Target);
@@ -165,8 +164,8 @@ public:
   Variable *getLinkedTo() const { return LinkedTo; }
   // Inherit dump() and emit() from Variable.
 private:
-  SpillVariable(Type Ty, SizeT Index, const IceString &Name)
-      : Variable(SpillVariableKind, Ty, Index, Name), LinkedTo(NULL) {}
+  SpillVariable(Type Ty, SizeT Index)
+      : Variable(SpillVariableKind, Ty, Index), LinkedTo(NULL) {}
   Variable *LinkedTo;
 };
 

src/IceOperand.cpp

@@ -131,16 +131,17 @@ void LiveRange::trim(InstNumberT Lower) {
     ++TrimmedBegin;
 }
 
-IceString Variable::getName() const {
-  if (!Name.empty())
-    return Name;
+IceString Variable::getName(const Cfg *Func) const {
+  if (Func && NameIndex >= 0)
+    return Func->getIdentifierName(NameIndex);
   return "__" + std::to_string(getIndex());
 }
 
 Variable Variable::asType(Type Ty) {
   // Note: This returns a Variable, even if the "this" object is a
   // subclass of Variable.
-  Variable V(kVariable, Ty, Number, Name);
+  Variable V(kVariable, Ty, Number);
+  V.NameIndex = NameIndex;
   V.RegNum = RegNum;
   V.StackOffset = StackOffset;
   return V;
@@ -405,12 +406,12 @@ void Variable::dump(const Cfg *Func, Ostream &Str) const {
   if (!ALLOW_DUMP)
     return;
   if (Func == NULL) {
-    Str << "%" << getName();
+    Str << "%" << getName(Func);
     return;
   }
   if (Func->getContext()->isVerbose(IceV_RegOrigins) ||
       (!hasReg() && !Func->getTarget()->hasComputedFrame()))
-    Str << "%" << getName();
+    Str << "%" << getName(Func);
   if (hasReg()) {
     if (Func->getContext()->isVerbose(IceV_RegOrigins))
       Str << ":";

src/IceOperand.h

@@ -388,18 +388,17 @@ class Variable : public Operand {
   Variable(Variable &&V) = default;
 
 public:
-  static Variable *create(Cfg *Func, Type Ty, SizeT Index,
-                          const IceString &Name) {
-    return new (Func->allocate<Variable>())
-        Variable(kVariable, Ty, Index, Name);
+  static Variable *create(Cfg *Func, Type Ty, SizeT Index) {
+    return new (Func->allocate<Variable>()) Variable(kVariable, Ty, Index);
   }
 
   SizeT getIndex() const { return Number; }
-  IceString getName() const;
-  void setName(IceString &NewName) {
+  IceString getName(const Cfg *Func) const;
+  void setName(Cfg *Func, const IceString &NewName) {
     // Make sure that the name can only be set once.
-    assert(Name.empty());
-    Name = NewName;
+    assert(NameIndex == Cfg::IdentifierIndexInvalid);
+    if (!NewName.empty())
+      NameIndex = Func->addIdentifierName(NewName);
   }
 
   bool getIsArg() const { return IsArgument; }
@@ -484,11 +483,11 @@ public:
   ~Variable() override {}
 
 protected:
-  Variable(OperandKind K, Type Ty, SizeT Index, const IceString &Name)
-      : Operand(K, Ty), Number(Index), Name(Name), IsArgument(false),
-        IsImplicitArgument(false), IgnoreLiveness(false), StackOffset(0),
-        RegNum(NoRegister), RegNumTmp(NoRegister), Weight(1), LoVar(NULL),
-        HiVar(NULL) {
+  Variable(OperandKind K, Type Ty, SizeT Index)
+      : Operand(K, Ty), Number(Index), NameIndex(Cfg::IdentifierIndexInvalid),
+        IsArgument(false), IsImplicitArgument(false), IgnoreLiveness(false),
+        StackOffset(0), RegNum(NoRegister), RegNumTmp(NoRegister), Weight(1),
+        LoVar(NULL), HiVar(NULL) {
     Vars = VarsReal;
     Vars[0] = this;
     NumVars = 1;
@@ -496,8 +495,7 @@ protected:
   // Number is unique across all variables, and is used as a
   // (bit)vector index for liveness analysis.
   const SizeT Number;
-  // Name is optional.
-  IceString Name;
+  Cfg::IdentifierIndexType NameIndex;
   bool IsArgument;
   bool IsImplicitArgument;
   // IgnoreLiveness means that the variable should be ignored when

src/IceRegAlloc.cpp

@@ -436,8 +436,9 @@ void LinearScan::scan(const llvm::SmallBitVector &RegMaskFull) {
           }
         }
         if (Verbose && Prefer) {
-          Str << "Initial Prefer=" << *Prefer << " R=" << PreferReg
-              << " LIVE=" << Prefer->getLiveRange()
+          Str << "Initial Prefer=";
+          Prefer->dump(Func);
+          Str << " R=" << PreferReg << " LIVE=" << Prefer->getLiveRange()
               << " Overlap=" << AllowOverlap << "\n";
         }
       }

src/IceTargetLoweringX8632.cpp

@@ -555,8 +555,9 @@ void TargetX8632::lowerArguments() {
     // to the assigned location of Arg.
     int32_t RegNum = RegX8632::Reg_xmm0 + NumXmmArgs;
     ++NumXmmArgs;
-    IceString Name = "home_reg:" + Arg->getName();
-    Variable *RegisterArg = Func->makeVariable(Ty, Name);
+    Variable *RegisterArg = Func->makeVariable(Ty);
+    if (ALLOW_DUMP)
+      RegisterArg->setName(Func, "home_reg:" + Arg->getName(Func));
     RegisterArg->setRegNum(RegNum);
     RegisterArg->setIsArg();
     Arg->setIsArg(false);
@@ -1050,8 +1051,12 @@ void TargetX8632::split64(Variable *Var) {
     return;
   }
   assert(Hi == NULL);
-  Lo = Func->makeVariable(IceType_i32, Var->getName() + "__lo");
-  Hi = Func->makeVariable(IceType_i32, Var->getName() + "__hi");
+  Lo = Func->makeVariable(IceType_i32);
+  Hi = Func->makeVariable(IceType_i32);
+  if (ALLOW_DUMP) {
+    Lo->setName(Func, Var->getName(Func) + "__lo");
+    Hi->setName(Func, Var->getName(Func) + "__hi");
+  }
   Var->setLoHi(Lo, Hi);
   if (Var->getIsArg()) {
     Lo->setIsArg();

src/PNaClTranslator.cpp

@@ -1086,8 +1086,6 @@ void ValuesymtabParser::ProcessRecord() {
   return;
 }
 
-class FunctionValuesymtabParser;
-
 /// Parses function blocks in the bitcode file.
 class FunctionParser : public BlockParserBaseClass {
   FunctionParser(const FunctionParser &) = delete;
@@ -2663,8 +2661,10 @@ void FunctionValuesymtabParser::setValueName(uint64_t Index, StringType &Name) {
     return;
   Ice::Operand *Op = getFunctionParser()->getOperand(Index);
   if (Ice::Variable *V = dyn_cast<Ice::Variable>(Op)) {
-    std::string Nm(Name.data(), Name.size());
-    V->setName(Nm);
+    if (ALLOW_DUMP) {
+      std::string Nm(Name.data(), Name.size());
+      V->setName(getFunctionParser()->getFunc(), Nm);
+    }
   } else {
     reportUnableToAssign("variable", Index, Name);
   }