Subzero: Remove Variable::NeedsStackSlot.

src/IceTargetLoweringX8632.cpp

Index: src/IceTargetLoweringX8632.cpp
diff --git a/src/IceTargetLoweringX8632.cpp b/src/IceTargetLoweringX8632.cpp
index ad8079293732d1830079d07f3ff185b7261d4c1f..a1e17389321420dd827e766290fae209d08ce30c 100644
--- a/src/IceTargetLoweringX8632.cpp
+++ b/src/IceTargetLoweringX8632.cpp
@@ -277,7 +277,7 @@ ICETYPE_TABLE;
 TargetX8632::TargetX8632(Cfg *Func)
     : TargetLowering(Func), InstructionSet(CLInstructionSet),
       IsEbpBasedFrame(false), NeedsStackAlignment(false), FrameSizeLocals(0),
-      SpillAreaSizeBytes(0), NextLabelNumber(0), ComputedLiveRanges(false) {
+      SpillAreaSizeBytes(0), NextLabelNumber(0) {
   // TODO: Don't initialize IntegerRegisters and friends every time.
   // Instead, initialize in some sort of static initializer for the
   // class.
@@ -373,7 +373,6 @@ void TargetX8632::translateO2() {
   // Validate the live range computations.  The expensive validation
   // call is deliberately only made when assertions are enabled.
   assert(Func->validateLiveness());
-  ComputedLiveRanges = true;
   // The post-codegen dump is done here, after liveness analysis and
   // associated cleanup, to make the dump cleaner and more useful.
   Func->dump("After initial x8632 codegen");
@@ -667,6 +666,28 @@ void TargetX8632::addProlog(CfgNode *Node) {
   //  * LocalsSpillAreaSize:    area 6
   //  * SpillAreaSizeBytes:     areas 3 - 7
 
+  // Make a final pass over the Cfg to determine which variables need
+  // stack slots.
+  llvm::BitVector NeedsStackSlot(Func->getNumVariables());
+  for (CfgNode *Node : Func->getNodes()) {
+    for (auto Inst = Node->getInsts().begin(), E = Node->getInsts().end();
+         Inst != E; ++Inst) {
+      if (Inst->isDeleted())
+        continue;

[jvoung (off chromium), 2014/11/11 17:18:09]

If the instruction isRedundantAssign() should this be skipped too?

[Jim Stichnoth, 2014/11/11 17:43:38]

No, isRedundantAssign() depends on the frame offsets being assigned, and at this
point that hasn't happened yet.

+      if (const Variable *Var = Inst->getDest())
+        NeedsStackSlot[Var->getIndex()] = true;

[jvoung (off chromium), 2014/11/11 17:18:10]

should this check if (!Var->hasReg()) first?

[Jim Stichnoth, 2014/11/11 17:43:39]

Good point.  It turns out that neither hasReg() check is actually necessary,
because the check is repeated (in line 741 of this version of the file).

I removed the check here, and renamed NeedsStackSlot to IsVarReferenced to
capture a more general meaning.

+      for (SizeT I = 0; I < Inst->getSrcSize(); ++I) {
+        Operand *Src = Inst->getSrc(I);
+        SizeT NumVars = Src->getNumVars();
+        for (SizeT J = 0; J < NumVars; ++J) {
+          const Variable *Var = Src->getVar(J);
+          if (!Var->hasReg())
+            NeedsStackSlot[Var->getIndex()] = true;
+        }
+      }
+    }
+  }
+
   // If SimpleCoalescing is false, each variable without a register
   // gets its own unique stack slot, which leads to large stack
   // frames.  If SimpleCoalescing is true, then each "global" variable
@@ -726,7 +747,7 @@ void TargetX8632::addProlog(CfgNode *Node) {
     if (Var->getIsArg())
       continue;
     // An unreferenced variable doesn't need a stack slot.
-    if (ComputedLiveRanges && !Var->needsStackSlot())
+    if (!NeedsStackSlot[Var->getIndex()])
       continue;
     // A spill slot linked to a variable with a stack slot should reuse
     // that stack slot.
@@ -4254,7 +4275,6 @@ void TargetX8632::lowerPhiAssignments(CfgNode *Node,
         RegNum = RegsForType.find_first();
         Preg = getPhysicalRegister(RegNum, Dest->getType());
         SpillLoc = Func->makeVariable(Dest->getType());
-        SpillLoc->setNeedsStackSlot();
         if (IsVector)
           _movp(SpillLoc, Preg);
         else