Subzero: Fix a register allocation issue for "advanced phi lowering".

src/IceTargetLoweringX8632.cpp

 //===- subzero/src/IceTargetLoweringX8632.cpp - x86-32 lowering -----------===//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the TargetLoweringX8632 class, which
@@ skipping 1673 matching lines @@
     Operand *Src0Lo = loOperand(Src0);
     Operand *Src0Hi = hiOperand(Src0);
     Variable *DestLo = llvm::cast<Variable>(loOperand(Dest));
     Variable *DestHi = llvm::cast<Variable>(hiOperand(Dest));
     Variable *T_Lo = nullptr, *T_Hi = nullptr;
     _mov(T_Lo, Src0Lo);
     _mov(DestLo, T_Lo);
     _mov(T_Hi, Src0Hi);
     _mov(DestHi, T_Hi);
   } else {
-    // If Dest is in memory, then RI is either a physical register or
-    // an immediate, otherwise RI can be anything.
-    Operand *RI =
-        legalize(Src0, Dest->hasReg() ? Legal_All : Legal_Reg | Legal_Imm);
+    Operand *RI;

[jvoung (off chromium), 2015/03/03 00:46:25]

Besides Phi, it does seem like this could affect load/store/bitcast also since
this is the generic lowerAssign(), so I wasn't too sure of the effect that this
would have. I guess worst case is, you get a dead assignment that's optimized
out.

[Jim Stichnoth, 2015/03/03 00:53:50]

For what it's worth, the asm code was the same before and after this change,
across Spec2k.  Yeah, the result is that the mov/movp below will be eliminated
as a redundant assignment.  However, assignments with precolored Dest operands
are rare: argument registers, return values, cl shift register, eax/edx
registers for mul/div, etc.

+    if (Dest->hasReg())
+      // If Dest already has a physical register, then legalize the
+      // Src operand into a Variable with the same register
+      // assignment.  This is mostly a workaround for advanced phi
+      // lowering's ad-hoc register allocation which assumes no
+      // register allocation is needed when at least one of the
+      // operands is non-memory.
+      RI = legalize(Src0, Legal_Reg, Dest->getRegNum());
+    else
+      // If Dest could be a stack operand, then RI must be a physical
+      // register or a scalar integer immediate.
+      RI = legalize(Src0, Legal_Reg | Legal_Imm);
     if (isVectorType(Dest->getType()))
       _movp(Dest, RI);
     else
       _mov(Dest, RI);
   }
 }
 
 void TargetX8632::lowerBr(const InstBr *Inst) {
   if (Inst->isUnconditional()) {
     _br(Inst->getTargetUnconditional());
@@ skipping 2448 matching lines @@
       Phi->setDeleted();
     }
   }
 }
 
 namespace {
 
 bool isMemoryOperand(const Operand *Opnd) {
   if (const auto Var = llvm::dyn_cast<Variable>(Opnd))
     return !Var->hasReg();
+  // We treat vector undef values the same as a memory operand,
+  // because they do in fact need a register to materialize the vector
+  // of zeroes into.
+  if (llvm::isa<ConstantUndef>(Opnd))
+    return isScalarFloatingType(Opnd->getType()) ||
+           isVectorType(Opnd->getType());
   if (llvm::isa<Constant>(Opnd))
     return isScalarFloatingType(Opnd->getType());
   return true;
 }
 
 } // end of anonymous namespace
 
 // Lower the pre-ordered list of assignments into mov instructions.
 // Also has to do some ad-hoc register allocation as necessary.
 void TargetX8632::lowerPhiAssignments(CfgNode *Node,
@@ skipping 597 matching lines @@
   case FT_Asm:
   case FT_Iasm: {
     OstreamLocker L(Ctx);
     emitConstantPool<PoolTypeConverter<float>>(Ctx);
     emitConstantPool<PoolTypeConverter<double>>(Ctx);
   } break;
   }
 }
 
 } // end of namespace Ice