Bug fixes and improvements to variable splitting

around loops.

Highlights of change:
* Use M0 emask without WriteEnable for copy in pre-header/loop exit whenever possible.
* Modified Register Pressure Estimator pass to not consider spilled variables. This is used to make splitting heuristic more accurate.
* Modified variable splitting cost heuristic based on experiments.
* Simplify removal of instruction inserted by split pass when split dcl spills in later RA iteration.
* Copy src modifier to new operand generated by split pass.

Author: pratikashar

visa/GraphColor.cpp

@@ -10739,7 +10739,11 @@ int GlobalRA::coloringRegAlloc()
                 if (iterationNo == 0 && !fastCompile &&
                     kernel.getOption(vISA_DoSplitOnSpill))
                 {
-                    LoopVarSplit loopSplit(kernel, &coloring, &rpe);
+                    if (builder.getOption(vISA_RATrace))
+                    {
+                        std::cout << "\t--var split around loop\n";
+                    }
+                    LoopVarSplit loopSplit(kernel, &coloring, &liveAnalysis);
                     kernel.fg.getLoops().computePreheaders();
                     loopSplit.run();
                 }

visa/RPE.cpp

@@ -14,10 +14,15 @@ SPDX-License-Identifier: MIT
 
 namespace vISA
 {
-    RPE::RPE(const GlobalRA& g, const LivenessAnalysis* l) : m(1024), gra(g), liveAnalysis(l), live(l->getNumSelectedVar()),
+    RPE::RPE(const GlobalRA& g, const LivenessAnalysis* l, DECLARE_LIST* spills) : m(1024), gra(g), liveAnalysis(l), live(l->getNumSelectedVar()),
         vars(l->vars)
     {
         options = g.kernel.getOptions();
+        if (spills)
+        {
+            std::for_each(spills->begin(), spills->end(),
+                [&](const G4_Declare* dcl) { spilledVars.insert(dcl); });
+        }
     }
 
     void RPE::run()
@@ -139,6 +144,8 @@ namespace vISA
             {
                 auto range = vars[i];
                 G4_Declare* rootDcl = range->getDeclare()->getRootDeclare();
+                if (isSpilled(rootDcl))
+                    continue;
                 if (rootDcl->getNumElems() > 1)
                 {
                     regPressure += rootDcl->getNumRows();
@@ -166,18 +173,21 @@ namespace vISA
         auto change = before^after;
         if (change)
         {
+            auto dcl = vars[id]->getDeclare();
+            if (isSpilled(dcl))
+                return;
             // For <1 GRF variable we have to take alignment into consideration as well when computing register pressure.
             // For now we double each <1GRF variable's size if its alignment also exceeds its size.
             // Alternative is to simply take the alignment as the size, but it might cause performance regressions
             // due to being too conservative (i.e., a GRF-aligned variable may share physical GRF with several other
-            auto dclSize = vars[id]->getDeclare()->getByteSize();
-            if (dclSize < gra.builder.getGRFSize() && dclSize < static_cast<uint32_t>(vars[id]->getDeclare()->getSubRegAlign()) * 2)
+            auto dclSize = dcl->getByteSize();
+            if (dclSize < gra.builder.getGRFSize() && dclSize < static_cast<uint32_t>(dcl->getSubRegAlign()) * 2)
             {
                 dclSize *= 2;
             }
 
             double delta = dclSize < gra.builder.getGRFSize() ?
-                dclSize / (double) gra.builder.getGRFSize() : (double) vars[id]->getDeclare()->getNumRows();
+                dclSize / (double) gra.builder.getGRFSize() : (double) dcl->getNumRows();
             if (before & change)
             {
                 if (regPressure < delta)

visa/RPE.h

@@ -18,7 +18,7 @@ namespace vISA
     class RPE
     {
     public:
-        RPE(const GlobalRA&, const LivenessAnalysis*);
+        RPE(const GlobalRA&, const LivenessAnalysis*, DECLARE_LIST* spills = nullptr);
 
         ~RPE()
         {
@@ -56,10 +56,24 @@ namespace vISA
         const Options* options;
         SparseBitSet live;
         const std::vector<G4_RegVar*>& vars;
+        // Variables part of spilledVars set dont contribute to
+        // program register pressure. This is useful to model
+        // register pressure immediately after coloring (spill
+        // iteration).
+        std::unordered_set<const G4_Declare*> spilledVars;
 
         void regPressureBBExit(G4_BB*);
         void updateRegisterPressure(unsigned int, unsigned int, unsigned int);
         void updateLiveness(SparseBitSet&, uint32_t, bool);
+
+
+        bool isSpilled(const G4_Declare* dcl) const
+        {
+            auto it = spilledVars.find(dcl);
+            if (it == spilledVars.end())
+                return false;
+            return true;
+        }
     };
 }
 #endif

visa/SpillManagerGMRF.cpp

@@ -3268,14 +3268,6 @@ void SpillManagerGRF::insertSpillRangeCode(
         return;
     }
 
-    if (builder_->getOption(vISA_DoSplitOnSpill))
-    {
-        // if spilled inst is copy of original variable to it's split variable
-        // then simply remove the instruction.
-        if (LoopVarSplit::removeFromPreheader(&gra, spillDcl, bb, spilledInstIter))
-            return;
-    }
-
     auto checkRMWNeeded = [this, spilledRegion]()
     {
         return noRMWNeeded.find(spilledRegion) == noRMWNeeded.end();
@@ -3553,22 +3545,6 @@ void SpillManagerGRF::insertFillGRFRangeCode(
     G4_INST* fillSendInst = nullptr;
     auto spillDcl = filledRegion->getTopDcl()->getRootDeclare();
 
-    if (builder_->getOption(vISA_DoSplitOnSpill))
-    {
-        // if spilled inst is copy of split variable to it's spilled variable
-        // then simply remove the instruction.
-        //
-        // if inst is:
-        // (W) mov (8|M0) SPLIT1    V10
-        //
-        // and SPLIT1 is marked as spilled then don't insert spill code for it.
-        // V10 is guaranteed to be spilled already so there is no point spilling
-        // SPLIT1. we simply remove above instruction and any fill emitted to load
-        // V10 and return.
-        if (LoopVarSplit::removeFromLoopExit(&gra, spillDcl, bb, filledInstIter))
-            return;
-    }
-
     auto sisIt = scalarImmSpill.find(spillDcl);
     if (sisIt != scalarImmSpill.end())
     {
@@ -4541,6 +4517,20 @@ bool SpillManagerGRF::insertSpillFillCode(
         return false;
     }
 
+    if (kernel->getOption(vISA_DoSplitOnSpill))
+    {
+        // remove all spilled splits
+        for (const LiveRange* lr : *spilledLRs_)
+        {
+            auto dcl = lr->getDcl();
+            // check whether spilled variable is one of split vars
+            if (gra.splitResults.find(dcl) == gra.splitResults.end())
+                continue;
+
+            LoopVarSplit::removeAllSplitInsts(&gra, dcl);
+        }
+    }
+
     // Insert spill/fill code for all basic blocks.
     updateRMWNeeded();
     FlowGraph& fg = kernel->fg;