Subzero: Change the way bitcast stack slot lowering is handled.

When doing a bitcast between int and FP types, the way lowering works
is that a spill temporary is created, with regalloc weight of zero to
inhibit register allocation, and this spill temporary is used for the
cvt instruction.  If the other variable does not get
register-allocated, then addProlog() forces the spill temporary to
share the same stack slot as the other variable.

Currently, the lowering code passes this information to addProlog()
by using the setPreferredRegister() mechanism.

This is changed by creating a target-specific subclass of Variable, so
that only the spill temporaries need to carry this extra information.

Ultimately, many of the existing Variable fields will be refactored
into a separate structure, and only generated/used as needed by
various optimization passes.  The spill temporary linkage is the one
thing that is still needed with Om1 when no optimizations are enabled,
motivating this change.

A couple other minor cleanups are also done here.

The key test is that the cast cross tests continue to work,
specifically the bitcast tests.

BUG= none
R=jvoung@chromium.org

Review URL: https://codereview.chromium.org/586943003

diff --git a/src/IceCfg.cpp b/src/IceCfg.cpp
index 4439323..a08bebb 100644 (file)
--- a/src/IceCfg.cpp
+++ b/src/IceCfg.cpp
@@ -45,13 +45,9 @@ CfgNode *Cfg::makeNode(const IceString &Name) {
   return Node;
 }
 
-// Create a new Variable with a particular type and an optional
-// name.  The Node argument is the node where the variable is defined.
 Variable *Cfg::makeVariable(Type Ty, const CfgNode *Node,
                             const IceString &Name) {
-  SizeT Index = Variables.size();
-  Variables.push_back(Variable::create(this, Ty, Node, Index, Name));
-  return Variables[Index];
+  return makeVariable<Variable>(Ty, Node, Name);
 }
 
 void Cfg::addArg(Variable *Arg) {
@@ -359,7 +355,7 @@ void Cfg::dump(const IceString &Message) {
     }
     Str << ") {\n";
   }
-  setCurrentNode(NULL);
+  resetCurrentNode();
   if (getContext()->isVerbose(IceV_Liveness)) {
     // Print summary info about variables
     for (VarList::const_iterator I = Variables.begin(), E = Variables.end();

diff --git a/src/IceCfg.h b/src/IceCfg.h
index a2582eb..7a265ad 100644 (file)
--- a/src/IceCfg.h
+++ b/src/IceCfg.h
@@ -61,6 +61,17 @@ public:
   InstNumberT newInstNumber() { return NextInstNumber++; }
 
   // 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>
+  T *makeVariable(Type Ty, const CfgNode *Node, const IceString &Name = "") {
+    SizeT Index = Variables.size();
+    T *Var = T::create(this, Ty, Node, Index, Name);
+    Variables.push_back(Var);
+    return Var;
+  }
+  // TODO(stichnot): Remove this function with C++11, and use default
+  // argument <typename T=Variable> above.
   Variable *makeVariable(Type Ty, const CfgNode *Node,
                          const IceString &Name = "");
   SizeT getNumVariables() const { return Variables.size(); }
@@ -99,6 +110,7 @@ public:
   // Manage the CurrentNode field, which is used for validating the
   // Variable::DefNode field during dumping/emitting.
   void setCurrentNode(const CfgNode *Node) { CurrentNode = Node; }
+  void resetCurrentNode() { setCurrentNode(NULL); }
   const CfgNode *getCurrentNode() const { return CurrentNode; }
 
   void emit();
@@ -155,8 +167,8 @@ private:
   // CurrentNode is maintained during dumping/emitting just for
   // validating Variable::DefNode.  Normally, a traversal over
   // CfgNodes maintains this, but before global operations like
-  // register allocation, setCurrentNode(NULL) should be called to
-  // avoid spurious validation failures.
+  // register allocation, resetCurrentNode() should be called to avoid
+  // spurious validation failures.
   const CfgNode *CurrentNode;
 
   Cfg(const Cfg &) LLVM_DELETED_FUNCTION;

diff --git a/src/IceCfgNode.cpp b/src/IceCfgNode.cpp
index 57aba6b..2b05e58 100644 (file)
--- a/src/IceCfgNode.cpp
+++ b/src/IceCfgNode.cpp
@@ -351,7 +351,7 @@ bool CfgNode::liveness(Liveness *Liveness) {
     // This is a fatal liveness consistency error.  Print some
     // diagnostics and abort.
     Ostream &Str = Func->getContext()->getStrDump();
-    Func->setCurrentNode(NULL);
+    Func->resetCurrentNode();
     Str << "LiveOrig-Live =";
     for (SizeT i = Live.size(); i < LiveOrig.size(); ++i) {
       if (LiveOrig.test(i)) {

diff --git a/src/IceInstX8632.h b/src/IceInstX8632.h
index a24f986..ac39170 100644 (file)
--- a/src/IceInstX8632.h
+++ b/src/IceInstX8632.h
@@ -135,6 +135,33 @@ private:
   Portion Part;
 };
 
+// SpillVariable decorates a Variable by linking it to another
+// Variable.  When stack frame offsets are computed, the SpillVariable
+// is given a distinct stack slot only if its linked Variable has a
+// register.  If the linked Variable has a stack slot, then the
+// Variable and SpillVariable share that slot.
+class SpillVariable : public Variable {
+public:
+  static SpillVariable *create(Cfg *Func, Type Ty, const CfgNode *Node,
+                               SizeT Index, const IceString &Name) {
+    return new (Func->allocate<SpillVariable>())
+        SpillVariable(Ty, Node, Index, Name);
+  }
+  const static OperandKind SpillVariableKind =
+      static_cast<OperandKind>(kVariable_Target);
+  static bool classof(const Operand *Operand) {
+    return Operand->getKind() == SpillVariableKind;
+  }
+  void setLinkedTo(Variable *Var) { LinkedTo = Var; }
+  Variable *getLinkedTo() const { return LinkedTo; }
+  // Inherit dump() and emit() from Variable.
+private:
+  SpillVariable(Type Ty, const CfgNode *Node, SizeT Index,
+                const IceString &Name)
+      : Variable(SpillVariableKind, Ty, Node, Index, Name), LinkedTo(NULL) {}
+  Variable *LinkedTo;
+};
+
 class InstX8632 : public InstTarget {
 public:
   enum InstKindX8632 {

diff --git a/src/IceOperand.cpp b/src/IceOperand.cpp
index 1f232dd..e4aaf24 100644 (file)
--- a/src/IceOperand.cpp
+++ b/src/IceOperand.cpp
@@ -189,7 +189,9 @@ IceString Variable::getName() const {
 }
 
 Variable Variable::asType(Type Ty) {
-  Variable V(Ty, DefNode, Number, Name);
+  // Note: This returns a Variable, even if the "this" object is a
+  // subclass of Variable.
+  Variable V(kVariable, Ty, DefNode, Number, Name);
   V.RegNum = RegNum;
   V.StackOffset = StackOffset;
   return V;

diff --git a/src/IceOperand.h b/src/IceOperand.h
index 02a1cf7..7fedfd2 100644 (file)
--- a/src/IceOperand.h
+++ b/src/IceOperand.h
@@ -25,6 +25,7 @@ namespace Ice {
 
 class Operand {
 public:
+  static const size_t MaxTargetKinds = 10;
   enum OperandKind {
     kConst_Base,
     kConstInteger32,
@@ -33,8 +34,11 @@ public:
     kConstDouble,
     kConstRelocatable,
     kConstUndef,
-    kConst_Num,
+    kConst_Target, // leave space for target-specific constant kinds
+    kConst_Num = kConst_Target + MaxTargetKinds,
     kVariable,
+    kVariable_Target, // leave space for target-specific variable kinds
+    kVariable_Num = kVariable_Target + MaxTargetKinds,
     // Target-specific operand classes use kTarget as the starting
     // point for their Kind enum space.
     kTarget
@@ -339,10 +343,14 @@ Ostream &operator<<(Ostream &Str, const LiveRange &L);
 // stack-allocated.  If it is register-allocated, it will ultimately
 // have a non-negative RegNum field.
 class Variable : public Operand {
+  Variable(const Variable &) LLVM_DELETED_FUNCTION;
+  Variable &operator=(const Variable &) LLVM_DELETED_FUNCTION;
+
 public:
   static Variable *create(Cfg *Func, Type Ty, const CfgNode *Node, SizeT Index,
                           const IceString &Name) {
-    return new (Func->allocate<Variable>()) Variable(Ty, Node, Index, Name);
+    return new (Func->allocate<Variable>())
+        Variable(kVariable, Ty, Node, Index, Name);
   }
 
   SizeT getIndex() const { return Number; }
@@ -431,16 +439,18 @@ public:
   virtual void dump(const Cfg *Func, Ostream &Str) const;
 
   static bool classof(const Operand *Operand) {
-    return Operand->getKind() == kVariable;
+    OperandKind Kind = Operand->getKind();
+    return Kind >= kVariable && Kind <= kVariable_Num;
   }
 
   // The destructor is public because of the asType() method.
   virtual ~Variable() {}
 
-private:
-  Variable(Type Ty, const CfgNode *Node, SizeT Index, const IceString &Name)
-      : Operand(kVariable, Ty), Number(Index), Name(Name), DefInst(NULL),
-        DefNode(Node), IsMultidef(false), IsArgument(false), StackOffset(0),
+protected:
+  Variable(OperandKind K, Type Ty, const CfgNode *Node, SizeT Index,
+           const IceString &Name)
+      : Operand(K, Ty), Number(Index), Name(Name), DefInst(NULL), DefNode(Node),
+        IsMultidef(false), IsArgument(false), StackOffset(0),
         RegNum(NoRegister), RegNumTmp(NoRegister), Weight(1),
         RegisterPreference(NULL), AllowRegisterOverlap(false), LoVar(NULL),
         HiVar(NULL) {
@@ -448,8 +458,6 @@ private:
     Vars[0] = this;
     NumVars = 1;
   }
-  Variable(const Variable &) LLVM_DELETED_FUNCTION;
-  Variable &operator=(const Variable &) LLVM_DELETED_FUNCTION;
   // Number is unique across all variables, and is used as a
   // (bit)vector index for liveness analysis.
   const SizeT Number;

diff --git a/src/IceRegAlloc.cpp b/src/IceRegAlloc.cpp
index ce08c6a..f79e8dc 100644 (file)
--- a/src/IceRegAlloc.cpp
+++ b/src/IceRegAlloc.cpp
@@ -29,7 +29,7 @@ namespace Ice {
 // two interfering variables to share the same register in certain
 // cases.
 //
-// Requires running Cfg::liveness(Liveness_RangesFull) in
+// Requires running Cfg::liveness(Liveness_Intervals) in
 // preparation.  Results are assigned to Variable::RegNum for each
 // Variable.
 void LinearScan::scan(const llvm::SmallBitVector &RegMaskFull) {
@@ -39,7 +39,7 @@ void LinearScan::scan(const llvm::SmallBitVector &RegMaskFull) {
   Inactive.clear();
   Active.clear();
   Ostream &Str = Func->getContext()->getStrDump();
-  Func->setCurrentNode(NULL);
+  Func->resetCurrentNode();
 
   // Gather the live ranges of all variables and add them to the
   // Unhandled set.  TODO: Unhandled is a set<> which is based on a
@@ -447,7 +447,7 @@ void LinearScan::dump(Cfg *Func) const {
   Ostream &Str = Func->getContext()->getStrDump();
   if (!Func->getContext()->isVerbose(IceV_LinearScan))
     return;
-  Func->setCurrentNode(NULL);
+  Func->resetCurrentNode();
   Str << "**** Current regalloc state:\n";
   Str << "++++++ Handled:\n";
   for (UnorderedRanges::const_iterator I = Handled.begin(), E = Handled.end();

diff --git a/src/IceTargetLoweringX8632.cpp b/src/IceTargetLoweringX8632.cpp
index d171739..f66a364 100644 (file)
--- a/src/IceTargetLoweringX8632.cpp
+++ b/src/IceTargetLoweringX8632.cpp
@@ -700,8 +700,7 @@ void TargetX8632::addProlog(CfgNode *Node) {
   RegsUsed = llvm::SmallBitVector(CalleeSaves.size());
   const VarList &Variables = Func->getVariables();
   const VarList &Args = Func->getArgs();
-  VarList SpilledVariables, SortedSpilledVariables,
-      VariablesLinkedToSpillSplots;
+  VarList SpilledVariables, SortedSpilledVariables, VariablesLinkedToSpillSlots;
 
   // If there is a separate locals area, this specifies the alignment
   // for it.
@@ -725,12 +724,11 @@ void TargetX8632::addProlog(CfgNode *Node) {
       continue;
     // A spill slot linked to a variable with a stack slot should reuse
     // that stack slot.
-    if (Var->getWeight() == RegWeight::Zero && Var->getRegisterOverlap()) {
-      if (Variable *Linked = Var->getPreferredRegister()) {
-        if (!Linked->hasReg()) {
-          VariablesLinkedToSpillSplots.push_back(Var);
-          continue;
-        }
+    if (SpillVariable *SpillVar = llvm::dyn_cast<SpillVariable>(Var)) {
+      assert(Var->getWeight() == RegWeight::Zero);
+      if (!SpillVar->getLinkedTo()->hasReg()) {
+        VariablesLinkedToSpillSlots.push_back(Var);
+        continue;
       }
     }
     SpilledVariables.push_back(Var);
@@ -878,11 +876,11 @@ void TargetX8632::addProlog(CfgNode *Node) {
 
   // Assign stack offsets to variables that have been linked to spilled
   // variables.
-  for (VarList::const_iterator I = VariablesLinkedToSpillSplots.begin(),
-                               E = VariablesLinkedToSpillSplots.end();
+  for (VarList::const_iterator I = VariablesLinkedToSpillSlots.begin(),
+                               E = VariablesLinkedToSpillSlots.end();
        I != E; ++I) {
     Variable *Var = *I;
-    Variable *Linked = Var->getPreferredRegister();
+    Variable *Linked = (llvm::cast<SpillVariable>(Var))->getLinkedTo();
     Var->setStackOffset(Linked->getStackOffset());
   }
 
@@ -2278,9 +2276,11 @@ void TargetX8632::lowerCast(const InstCast *Inst) {
       Variable *T = NULL;
       // TODO: Should be able to force a spill setup by calling legalize() with
       // Legal_Mem and not Legal_Reg or Legal_Imm.
-      Variable *Spill = Func->makeVariable(SrcType, Context.getNode());
+      SpillVariable *SpillVar =
+          Func->makeVariable<SpillVariable>(SrcType, Context.getNode());
+      SpillVar->setLinkedTo(Dest);
+      Variable *Spill = SpillVar;
       Spill->setWeight(RegWeight::Zero);
-      Spill->setPreferredRegister(Dest, true);
       _mov(T, Src0RM);
       _mov(Spill, T);
       _mov(Dest, Spill);
@@ -2294,9 +2294,11 @@ void TargetX8632::lowerCast(const InstCast *Inst) {
       //   a_lo.i32 = t_lo.i32
       //   t_hi.i32 = hi(s.f64)
       //   a_hi.i32 = t_hi.i32
-      Variable *Spill = Func->makeVariable(IceType_f64, Context.getNode());
+      SpillVariable *SpillVar =
+          Func->makeVariable<SpillVariable>(IceType_f64, Context.getNode());
+      SpillVar->setLinkedTo(llvm::dyn_cast<Variable>(Src0RM));
+      Variable *Spill = SpillVar;
       Spill->setWeight(RegWeight::Zero);
-      Spill->setPreferredRegister(llvm::dyn_cast<Variable>(Src0RM), true);
       _movq(Spill, Src0RM);
 
       Variable *DestLo = llvm::cast<Variable>(loOperand(Dest));
@@ -2323,9 +2325,11 @@ void TargetX8632::lowerCast(const InstCast *Inst) {
       //   t_hi.i32 = b_hi.i32
       //   hi(s.f64) = t_hi.i32
       //   a.f64 = s.f64
-      Variable *Spill = Func->makeVariable(IceType_f64, Context.getNode());
+      SpillVariable *SpillVar =
+          Func->makeVariable<SpillVariable>(IceType_f64, Context.getNode());
+      SpillVar->setLinkedTo(Dest);
+      Variable *Spill = SpillVar;
       Spill->setWeight(RegWeight::Zero);
-      Spill->setPreferredRegister(Dest, true);
 
       Variable *T_Lo = NULL, *T_Hi = NULL;
       VariableSplit *SpillLo =
@@ -3720,7 +3724,7 @@ bool matchOffsetBase(Variable *&Base, int32_t &Offset, const Inst *&Reason) {
 
 void computeAddressOpt(Cfg *Func, const Inst *Instr, Variable *&Base,
                        Variable *&Index, uint16_t &Shift, int32_t &Offset) {
-  Func->setCurrentNode(NULL);
+  Func->resetCurrentNode();
   if (Func->getContext()->isVerbose(IceV_AddrOpt)) {
     Ostream &Str = Func->getContext()->getStrDump();
     Str << "\nStarting computeAddressOpt for instruction:\n  ";