Subzero: Fix a bug in register allocator overlap computation.

src/IceCfg.cpp

@@ -810,6 +810,22 @@ void Cfg::dump(const IceString &Message) {
         Str << getVMetadata()->isMultiBlock(Var);
       else
         Str << "?";
+      Str << " defs=";
+      bool FirstPrint = true;
+      if (VMetadata->getKind() != VMK_Uses) {
+        if (const Inst *FirstDef = VMetadata->getFirstDefinition(Var)) {
+          Str << FirstDef->getNumber();
+          FirstPrint = false;
+        }
+      }
+      if (VMetadata->getKind() == VMK_All) {
+        for (const Inst *Instr : VMetadata->getLatterDefinitions(Var)) {
+          if (!FirstPrint)
+            Str << ",";
+          Str << Instr->getNumber();
+          FirstPrint = false;
+        }
+      }
       Str << " weight=" << Var->getWeight(this) << " ";
       Var->dump(this);
       Str << " LIVE=" << Var->getLiveRange() << "\n";

src/IceCompileServer.cpp

@@ -94,8 +94,7 @@ ErrorCodes getReturnValue(const Ice::ClFlagsExtra &Flags, ErrorCodes Val) {
 }
 
 // Reports fatal error message, and then exits with success status 0.
-void reportFatalErrorThenExitSuccess(void * UserData,
-                                     const std::string &Reason,
+void reportFatalErrorThenExitSuccess(void *UserData, const std::string &Reason,
                                      bool GenCrashDag) {
   (void)UserData;
   (void)GenCrashDag;

src/IceOperand.cpp

@@ -213,15 +213,16 @@ void VariableTracking::markDef(MetadataKind TrackingKind, const Inst *Instr,
   // omit all uses of the variable if markDef() and markUse() both use this
   // optimization.
   assert(Node);
-// Verify that instructions are added in increasing order.
-#ifndef NDEBUG
-  if (TrackingKind == VMK_All) {
-    const Inst *LastInstruction =
-        Definitions.empty() ? FirstOrSingleDefinition : Definitions.back();
-    assert(LastInstruction == nullptr ||
-           Instr->getNumber() >= LastInstruction->getNumber());
+  // Verify that instructions are added in increasing order.
+  if (BuildDefs::asserts()) {
+    if (TrackingKind == VMK_All) {
+      const Inst *LastInstruction =
+          Definitions.empty() ? FirstOrSingleDefinition : Definitions.back();
+      (void)LastInstruction;
+      assert(LastInstruction == nullptr ||
+             Instr->getNumber() >= LastInstruction->getNumber());
+    }
   }
-#endif
   constexpr bool IsImplicit = false;
   markUse(TrackingKind, Instr, Node, IsImplicit);
   if (TrackingKind == VMK_Uses)
@@ -258,7 +259,7 @@ void VariableTracking::markDef(MetadataKind TrackingKind, const Inst *Instr,
   }
 }
 
-const Inst *VariableTracking::getFirstDefinition() const {
+const Inst *VariableTracking::getFirstDefinitionSingleBlock() const {
   switch (MultiDef) {
   case MDS_Unknown:
   case MDS_MultiDefMultiBlock:
@@ -284,6 +285,19 @@ const Inst *VariableTracking::getSingleDefinition() const {
   return nullptr;
 }
 
+const Inst *VariableTracking::getFirstDefinition() const {
+  switch (MultiDef) {
+  case MDS_Unknown:
+    return nullptr;
+  case MDS_MultiDefMultiBlock:
+  case MDS_SingleDef:
+  case MDS_MultiDefSingleBlock:
+    assert(FirstOrSingleDefinition);
+    return FirstOrSingleDefinition;
+  }
+  return nullptr;
+}
+
 void VariablesMetadata::init(MetadataKind TrackingKind) {
   TimerMarker T(TimerStack::TT_vmetadata, Func);
   Kind = TrackingKind;
@@ -303,7 +317,7 @@ void VariablesMetadata::init(MetadataKind TrackingKind) {
 }
 
 void VariablesMetadata::addNode(CfgNode *Node) {
-  if (Func->getNumVariables() >= Metadata.size())
+  if (Func->getNumVariables() > Metadata.size())
     Metadata.resize(Func->getNumVariables());
 
   for (Inst &I : Node->getPhis()) {
@@ -364,12 +378,13 @@ bool VariablesMetadata::isMultiBlock(const Variable *Var) const {
   return Metadata[VarNum].getMultiBlock() != VariableTracking::MBS_SingleBlock;
 }
 
-const Inst *VariablesMetadata::getFirstDefinition(const Variable *Var) const {
+const Inst *
+VariablesMetadata::getFirstDefinitionSingleBlock(const Variable *Var) const {
   assert(Kind != VMK_Uses);
   if (!isTracked(Var))
     return nullptr; // conservative answer
   SizeT VarNum = Var->getIndex();
-  return Metadata[VarNum].getFirstDefinition();
+  return Metadata[VarNum].getFirstDefinitionSingleBlock();
 }
 
 const Inst *VariablesMetadata::getSingleDefinition(const Variable *Var) const {
@@ -380,6 +395,14 @@ const Inst *VariablesMetadata::getSingleDefinition(const Variable *Var) const {
   return Metadata[VarNum].getSingleDefinition();
 }
 
+const Inst *VariablesMetadata::getFirstDefinition(const Variable *Var) const {
+  assert(Kind != VMK_Uses);
+  if (!isTracked(Var))
+    return nullptr; // conservative answer
+  SizeT VarNum = Var->getIndex();
+  return Metadata[VarNum].getFirstDefinition();
+}
+
 const InstDefList &
 VariablesMetadata::getLatterDefinitions(const Variable *Var) const {
   assert(Kind == VMK_All);

src/IceOperand.h

@@ -629,8 +629,9 @@ public:
   VariableTracking(const VariableTracking &) = default;
   MultiDefState getMultiDef() const { return MultiDef; }
   MultiBlockState getMultiBlock() const { return MultiBlock; }
-  const Inst *getFirstDefinition() const;
+  const Inst *getFirstDefinitionSingleBlock() const;
   const Inst *getSingleDefinition() const;
+  const Inst *getFirstDefinition() const;
   const InstDefList &getLatterDefinitions() const { return Definitions; }
   CfgNode *getNode() const { return SingleUseNode; }
   RegWeight getUseWeight() const { return UseWeight; }
@@ -643,11 +644,10 @@ private:
   MultiBlockState MultiBlock = MBS_Unknown;
   CfgNode *SingleUseNode = nullptr;
   CfgNode *SingleDefNode = nullptr;
-  /// All definitions of the variable are collected here, in increasing
-  /// order of instruction number.
+  /// All definitions of the variable are collected in Definitions[] (except for
+  /// the earliest definition), in increasing order of instruction number.
   InstDefList Definitions; /// Only used if Kind==VMK_All
-  const Inst *FirstOrSingleDefinition =
-      nullptr; /// Is a copy of Definitions[0] if Kind==VMK_All
+  const Inst *FirstOrSingleDefinition = nullptr;
   RegWeight UseWeight;
 };
 
@@ -665,6 +665,7 @@ public:
   /// Add a single node. This is called by init(), and can be called
   /// incrementally from elsewhere, e.g. after edge-splitting.
   void addNode(CfgNode *Node);
+  MetadataKind getKind() const { return Kind; }
   /// Returns whether the given Variable is tracked in this object. It should
   /// only return false if changes were made to the CFG after running init(), in
   /// which case the state is stale and the results shouldn't be trusted (but it
@@ -678,13 +679,17 @@ public:
   /// Returns the first definition instruction of the given Variable. This is
   /// only valid for variables whose definitions are all within the same block,
   /// e.g. T after the lowered sequence "T=B; T+=C; A=T", for which
-  /// getFirstDefinition(T) would return the "T=B" instruction. For variables
-  /// with definitions span multiple blocks, nullptr is returned.
-  const Inst *getFirstDefinition(const Variable *Var) const;
+  /// getFirstDefinitionSingleBlock(T) would return the "T=B" instruction. For
+  /// variables with definitions span multiple blocks, nullptr is returned.
+  const Inst *getFirstDefinitionSingleBlock(const Variable *Var) const;
   /// Returns the definition instruction of the given Variable, when the
   /// variable has exactly one definition. Otherwise, nullptr is returned.
   const Inst *getSingleDefinition(const Variable *Var) const;
-  /// Returns the list of all definition instructions of the given Variable.
+  /// getFirstDefinition() and getLatterDefinitions() are used together to
+  /// return the complete set of instructions that define the given Variable,
+  /// regardless of whether the definitions are within the same block (in
+  /// contrast to getFirstDefinitionSingleBlock).
+  const Inst *getFirstDefinition(const Variable *Var) const;
   const InstDefList &getLatterDefinitions(const Variable *Var) const;
 
   /// Returns whether the given Variable is live across multiple blocks. Mainly,

src/IceRegAlloc.cpp

@@ -38,9 +38,8 @@ bool overlapsDefs(const Cfg *Func, const Variable *Item, const Variable *Var) {
   if (const Inst *FirstDef = VMetadata->getFirstDefinition(Var))
     if (Item->getLiveRange().overlapsInst(FirstDef->getNumber(), UseTrimmed))
       return true;
-  const InstDefList &Defs = VMetadata->getLatterDefinitions(Var);
-  for (size_t i = 0; i < Defs.size(); ++i) {
-    if (Item->getLiveRange().overlapsInst(Defs[i]->getNumber(), UseTrimmed))
+  for (const Inst *Def : VMetadata->getLatterDefinitions(Var)) {
+    if (Item->getLiveRange().overlapsInst(Def->getNumber(), UseTrimmed))
       return true;
   }
   return false;
@@ -463,7 +462,8 @@ void LinearScan::findRegisterPreference(IterationState &Iter) {
 
   if (FindPreference) {
     VariablesMetadata *VMetadata = Func->getVMetadata();
-    if (const Inst *DefInst = VMetadata->getFirstDefinition(Iter.Cur)) {
+    if (const Inst *DefInst =
+            VMetadata->getFirstDefinitionSingleBlock(Iter.Cur)) {
       assert(DefInst->getDest() == Iter.Cur);
       bool IsAssign = DefInst->isSimpleAssign();
       bool IsSingleDef = !VMetadata->isMultiDef(Iter.Cur);

src/PNaClTranslator.cpp

@@ -2051,13 +2051,12 @@ private:
     return Context->getGlobalConstantByID(0);
   }
 
-  void verifyCallArgTypeMatches(Ice::FunctionDeclaration *Fcn,
-                                Ice::SizeT Index, Ice::Type ArgType,
-                                Ice::Type ParamType) {
+  void verifyCallArgTypeMatches(Ice::FunctionDeclaration *Fcn, Ice::SizeT Index,
+                                Ice::Type ArgType, Ice::Type ParamType) {
     if (ArgType != ParamType) {
       std::string Buffer;
       raw_string_ostream StrBuf(Buffer);
-      StrBuf << "Argument " << (Index + 1)  << " of " << printName(Fcn)
+      StrBuf << "Argument " << (Index + 1) << " of " << printName(Fcn)
              << " expects " << ParamType << ". Found: " << ArgType;
       Error(StrBuf.str());
     }
@@ -2733,7 +2732,7 @@ void FunctionParser::ProcessRecord() {
       return;
 
     // Extract out the the call parameters.
-    SmallVector<Ice::Operand*, 8> Params;
+    SmallVector<Ice::Operand *, 8> Params;
     for (Ice::SizeT Index = ParamsStartIndex; Index < Values.size(); ++Index) {
       Ice::Operand *Op = getRelativeOperand(Values[Index], BaseIndex);
       if (isIRGenerationDisabled())
@@ -2741,8 +2740,8 @@ void FunctionParser::ProcessRecord() {
       if (Op == nullptr) {
         std::string Buffer;
         raw_string_ostream StrBuf(Buffer);
-        StrBuf << "Parameter " << (Index - ParamsStartIndex + 1)
-               << " of " << printName(Fcn) << " is not defined";
+        StrBuf << "Parameter " << (Index - ParamsStartIndex + 1) << " of "
+               << printName(Fcn) << " is not defined";
         Error(StrBuf.str());
         if (ReturnType != Ice::IceType_void)
           setNextLocalInstIndex(nullptr);
@@ -2755,8 +2754,8 @@ void FunctionParser::ProcessRecord() {
     if (IntrinsicInfo == nullptr && !isCallReturnType(ReturnType)) {
       std::string Buffer;
       raw_string_ostream StrBuf(Buffer);
-      StrBuf << "Return type of " << printName(Fcn) << " is invalid: "
-             << ReturnType;
+      StrBuf << "Return type of " << printName(Fcn)
+             << " is invalid: " << ReturnType;
       Error(StrBuf.str());
       ReturnType = Ice::IceType_i32;
     }
@@ -2772,8 +2771,8 @@ void FunctionParser::ProcessRecord() {
       Ice::Operand *Op = Params[Index];
       Ice::Type OpType = Op->getType();
       if (Signature)
-        verifyCallArgTypeMatches(
-            Fcn, Index, OpType, Signature->getArgType(Index));
+        verifyCallArgTypeMatches(Fcn, Index, OpType,
+                                 Signature->getArgType(Index));
       if (IntrinsicInfo) {
         verifyCallArgTypeMatches(Fcn, Index, OpType,
                                  IntrinsicInfo->getArgType(Index));