Fix in assignColors to mark spills correctly. Fix in VarSplitPass to

keep order of intrinsic insertion deterministic. Minor fix in split cost
heuristic.

Change-Id: If72d58f945179d1a6b145817723acbed1ca44894

Author: pratikashar

visa/GraphColor.cpp

@@ -5745,7 +5745,7 @@ bool GraphColor::assignColors(ColorHeuristic colorHeuristicGRF, bool doBankConfl
 
     auto& varSplitPass = *gra.getVarSplitPass();
 
-    auto assignColor = [&](LiveRange* lr, bool ignoreChildrenIntf = false)
+    auto assignColor = [&](LiveRange* lr, bool ignoreChildrenIntf = false, bool spillAllowed = true)
     {
         auto lrVar = lr->getVar();
 
@@ -5925,7 +5925,13 @@ bool GraphColor::assignColors(ColorHeuristic colorHeuristicGRF, bool doBankConfl
                 else
                 {
                     // for first-fit register assignment track spilled live ranges
-                    spilledLRs.push_back(lr);
+                    if (spillAllowed)
+                    {
+                        // When retrying a coalesceable assignment, dont spill
+                        // if there is no GRF available.
+                        spilledLRs.push_back(lr);
+                        lr->setSpilled(true);
+                    }
                 }
             }
             else
@@ -5949,6 +5955,11 @@ bool GraphColor::assignColors(ColorHeuristic colorHeuristicGRF, bool doBankConfl
     for (auto iter = colorOrder.rbegin(), iterEnd = colorOrder.rend(); iter != iterEnd; ++iter)
     {
         auto lr = (*iter);
+
+        // in case child/parent was already spilled earlier, dont recolor
+        if(lr->isSpilled())
+            continue;
+
         bool ret = assignColor(lr);
 
         // early exit
@@ -5969,7 +5980,8 @@ bool GraphColor::assignColors(ColorHeuristic colorHeuristicGRF, bool doBankConfl
                         auto childLR = lrs[child->getRegVar()->getId()];
                         if (!childLR->getPhyReg())
                         {
-                            assignColor(childLR);
+                            auto isChildSpilled = childLR->isSpilled();
+                            assignColor(childLR, false, !isChildSpilled);
                         }
                         else
                         {
@@ -5980,7 +5992,7 @@ bool GraphColor::assignColors(ColorHeuristic colorHeuristicGRF, bool doBankConfl
                             if (oldPhyReg->asGreg()->getRegNum() == hint)
                                 continue;
                             childLR->resetPhyReg();
-                            bool success = assignColor(childLR);
+                            bool success = assignColor(childLR, false, false);
                             if (!success || childLR->getPhyReg()->asGreg()->getRegNum() != hint)
                                 childLR->setPhyReg(oldPhyReg, oldPhySubReg);
                         }
@@ -5997,16 +6009,24 @@ bool GraphColor::assignColors(ColorHeuristic colorHeuristicGRF, bool doBankConfl
                 auto parentLR = getLiveRanges()[parentDcl->getRegVar()->getId()];
                 auto oldPhyReg = parentLR->getPhyReg();
                 auto oldPhySubReg = parentLR->getPhyRegOff();
+                bool isParentSpilled = parentLR->isSpilled();
                 parentLR->resetPhyReg();
                 varSplitPass.writeHints(lr->getDcl(), getLiveRanges());
-                assignColor(parentLR, true);
+                assignColor(parentLR, true, !isParentSpilled);
                 // If parent's assigned GRF is non-coalesceable assignment then
                 // undo it as it is risky to keep this because parent's intf
                 // doesnt include children.
                 auto newParentAssignment = parentLR->getPhyReg();
                 if ((newParentAssignment && newParentAssignment->asGreg()->getRegNum() != parentLR->getAllocHint()) ||
                     !newParentAssignment)
                     parentLR->setPhyReg(oldPhyReg, oldPhySubReg);
+
+                if (isParentSpilled && parentLR->getPhyReg())
+                {
+                    // remove parent from spill list since it got an allocation this time
+                    spilledLRs.remove(parentLR);
+                    parentLR->setSpilled(false);
+                }
             }
         }
     }
@@ -6024,6 +6044,34 @@ bool GraphColor::assignColors(ColorHeuristic colorHeuristicGRF, bool doBankConfl
         }
     }
 
+#ifdef _DEBUG
+    // Verify that spilledLRs has no duplicate
+    for (auto item : spilledLRs)
+    {
+        unsigned int count = 0;
+        for (auto checkItem : spilledLRs)
+        {
+            if (checkItem == item)
+            {
+                MUST_BE_TRUE(count == 0, "Duplicate entry found in spilledLRs");
+                count++;
+            }
+        }
+    }
+
+    // Verify that none of spilledLRs have an allocation
+    for (auto lr : spilledLRs)
+    {
+        MUST_BE_TRUE(lr->getPhyReg() == nullptr, "Spilled LR contains valid allocation");
+    }
+
+    // Verify that all spilled LRs are synced
+    for (auto lr : spilledLRs)
+    {
+        MUST_BE_TRUE(lr->isSpilled(), "LR not marked as spilled, but inserted in spilledLRs list");
+    }
+#endif
+
     return true;
 }
 

visa/GraphColor.h

@@ -83,6 +83,7 @@ class LiveRange
     G4_RegFileKind regKind;
     bool* forbidden = nullptr;
     int numForbidden = -1;
+    bool spilled = false;
 
     GlobalRA& gra;
     unsigned numRegNeeded;
@@ -245,6 +246,9 @@ class LiveRange
         void setRetIp() { retIp = true; }
         bool isRetIp() const { return retIp; }
 
+        bool isSpilled() const { return spilled; }
+        void setSpilled(bool v) { spilled = v; }
+
 private:
     //const Options *m_options;
     unsigned getForbiddenVectorSize();

visa/VarSplit.cpp

@@ -328,10 +328,10 @@ void VarSplitPass::findSplitCandidates()
                 }
             }
 
-            // split if def-first use distance > 8
-            if (!split &&
-                (instId[item.second.srcs.front().first->getInst()] - instId[item.second.def.first->getInst()]) > 8)
-                split = true;
+            // dont split if def-last first use distance <= 8
+            if (split &&
+                (instId[item.second.srcs.back().first->getInst()] - instId[item.second.def.first->getInst()]) <= 8)
+                split = false;
 
             if (!split)
             {
@@ -361,8 +361,14 @@ void VarSplitPass::split()
     unsigned int numIntrinsicsInserted = 0;
 #endif
     // Do actual splitting
-    for (auto& item : splitVars)
+    for(auto curDcl : kernel.Declares)
     {
+        auto isCandidate = splitVars.find(curDcl);
+        if (isCandidate == splitVars.end())
+            continue;
+
+        auto item = *isCandidate;
+
         MUST_BE_TRUE(item.second.legitCandidate, "Cannot split non-candidate");
 
         // Insert intrinsics
@@ -406,6 +412,7 @@ void VarSplitPass::split()
             unsigned int esize = (getGRFSize() / G4_Type_Table[Type_UD].byteSize) * numRows;
             auto intrin = kernel.fg.builder->createIntrinsicInst(nullptr, Intrinsic::Split, esize, dstRgn, srcRgn, nullptr, nullptr,
                 item.second.def.first->getInst()->getOption() | G4_InstOption::InstOpt_WriteEnable, item.second.def.first->getInst()->getLineNo());
+            intrin->setCISAOff(item.second.def.first->getInst()->getCISAOff());
             item.second.def.second->insert(it, intrin);
             splitDcls.push_back(std::make_tuple(lb, rb, splitDcl));
 #ifdef DEBUG_VERBOSE_ON