Fix an incorrect condition when deciding if an unreferenced dcl can be

removed from list.

Author: pratikashar

IGC/Compiler/CISACodeGen/CISABuilder.cpp

@@ -3869,7 +3869,7 @@ namespace IGC
         }
 
         if ((context->type == ShaderType::OPENCL_SHADER || context->type == ShaderType::COMPUTE_SHADER) &&
-            VISAPlatform >= GENX_SKL && IGC_IS_FLAG_ENABLED(EnablePreemption))
+            m_program->m_Platform->preemptionSupported() && IGC_IS_FLAG_ENABLED(EnablePreemption))
         {
             SaveOption(vISA_enablePreemption, true);
         }

IGC/Compiler/CISACodeGen/Platform.hpp

@@ -711,6 +711,13 @@ bool supportHeaderRTW() const
 {
     return true;
 }
+
+bool preemptionSupported() const
+{
+
+    return GetPlatformFamily() >= IGFX_GEN9_CORE;
+};
+
 };
 
 }//namespace IGC

visa/BuildIRImpl.cpp

@@ -675,23 +675,20 @@ void IR_Builder::createPreDefinedVars()
 
 void IR_Builder::createBuiltinDecls()
 {
-
+    // realR0 is always tied to physical r0
     auto numR0DW = numEltPerGRF<Type_UD>();
-    builtinR0 = createDeclareNoLookup(
-        "BuiltinR0",
+    realR0 = createDeclareNoLookup(
+        "BuiltInR0",
         G4_INPUT,
         numR0DW,
         1,
         Type_UD);
-    builtinR0->getRegVar()->setPhyReg(phyregpool.getGreg(0), 0);
-    realR0 = builtinR0;
+    realR0->getRegVar()->setPhyReg(phyregpool.getGreg(0), 0);
 
-    if (m_options->getOption(vISA_enablePreemption))
-    {
-        G4_Declare *R0CopyDcl = createTempVar(numR0DW, Type_UD, GRFALIGN);
-        builtinR0 = R0CopyDcl;
-        R0CopyDcl->setDoNotSpill();
-    }
+    // builtinR0 either gets allocated to r0 or to a different
+    // register depending on conditions in RA.
+    builtinR0 = createTempVar(numR0DW, Type_UD, GRFALIGN, "R0_Copy");
+    builtinR0->setDoNotSpill();
 
     builtinA0 = createDeclareNoLookup(
         "BuiltinA0",

visa/GraphColor.cpp

@@ -9545,11 +9545,8 @@ int GlobalRA::coloringRegAlloc()
 
         // bind builtinR0 to the reserved stack call ABI GRF so that caller and
         // callee can agree on which GRF to use for r0
-        if (builder.getOption(vISA_enablePreemption))
-        {
-            builder.getBuiltinR0()->getRegVar()->setPhyReg(
-                builder.phyregpool.getGreg(kernel.getThreadHeaderGRF()), 0);
-        }
+        builder.getBuiltinR0()->getRegVar()->setPhyReg(
+            builder.phyregpool.getGreg(kernel.getThreadHeaderGRF()), 0);
     }
 
     if (!isReRAPass())

visa/LinearScanRA.cpp

@@ -785,7 +785,6 @@ void LinearScanRA::preRAAnalysis()
         const Options* opt = builder.getOptions();
         if (kernel.getInt32KernelAttr(Attributes::ATTR_Target) != VISA_3D ||
             opt->getOption(vISA_enablePreemption) ||
-            (kernel.fg.getHasStackCalls() || kernel.fg.getIsStackCallFunc()) ||
             opt->getOption(vISA_ReserveR0))
         {
             pregs->setR0Forbidden();

visa/LocalRA.cpp

@@ -298,7 +298,6 @@ void LocalRA::preLocalRAAnalysis()
         const Options *opt = builder.getOptions();
         if (kernel.getInt32KernelAttr(Attributes::ATTR_Target) != VISA_3D ||
             opt->getOption(vISA_enablePreemption) ||
-            stackCallRegSize > 0 ||
             opt->getOption(vISA_ReserveR0))
         {
             pregs->setR0Forbidden();
@@ -998,8 +997,7 @@ void GlobalRA::removeUnreferencedDcls()
         return (dcl->getRegFile() == G4_GRF || dcl->getRegFile() == G4_INPUT) &&
             getNumRefs(dcl) == 0 &&
             dcl->getRegVar()->isPhyRegAssigned() == false &&
-            !(kernel.fg.builder->getOption(vISA_enablePreemption) &&
-                dcl == kernel.fg.builder->getBuiltinR0())
+            dcl != kernel.fg.builder->getBuiltinR0()
             ;
     };
 

visa/Optimizer.cpp

@@ -168,6 +168,17 @@ void Optimizer::regAlloc()
 
     fg.prepareTraversal();
 
+    // realR0 and BuiltInR0 are 2 different dcls.
+    // realR0 is always tied to physical r0.
+    // if copy of r0 isnt needed then set latter to r0 as well.
+    // if copy of r0 is required, then let RA decide allocation of BuiltInR0.
+    if (!R0CopyNeeded())
+    {
+        // when no copy is needed, make BuiltInR0 an alias of realR0
+        builder.getBuiltinR0()->setAliasDeclare(builder.getRealR0(), 0);
+        builder.getBuiltinR0()->getRegVar()->setPhyReg(builder.getRealR0()->getRegVar()->getPhyReg(), 0);
+    }
+
     //
     // assign registers
     //
@@ -1022,7 +1033,7 @@ void Optimizer::initOptimizations()
     INITIALIZE_PASS(dumpPayload,             vISA_dumpPayload,             TimerID::MISC_OPTS);
     INITIALIZE_PASS(normalizeRegion,         vISA_EnableAlways,            TimerID::MISC_OPTS);
     INITIALIZE_PASS(collectStats,            vISA_EnableAlways,            TimerID::MISC_OPTS);
-    INITIALIZE_PASS(createR0Copy,            vISA_enablePreemption,        TimerID::MISC_OPTS);
+    INITIALIZE_PASS(createR0Copy,            vISA_EnableAlways,            TimerID::MISC_OPTS);
     INITIALIZE_PASS(initializePayload,       vISA_InitPayload,             TimerID::NUM_TIMERS);
     INITIALIZE_PASS(cleanupBindless,         vISA_enableCleanupBindless,   TimerID::OPTIMIZER);
     INITIALIZE_PASS(countGRFUsage,           vISA_PrintRegUsage,           TimerID::MISC_OPTS);
@@ -1416,6 +1427,23 @@ void Optimizer::accSubPostSchedule()
     accSub.run();
 }
 
+bool Optimizer::R0CopyNeeded()
+{
+    if (kernel.getOption(vISA_enablePreemption))
+    {
+        return true;
+    }
+
+    if (builder.getIsKernel() && kernel.fg.getHasStackCalls())
+    {
+        // As per VISA ABI, last register in GRF file should
+        // contain copy of r0.
+        return true;
+    }
+
+    return false;
+}
+
 int Optimizer::optimization()
 {
     // remove redundant message headers.
@@ -7428,6 +7456,12 @@ bool Optimizer::foldPseudoAndOr(G4_BB* bb, INST_LIST_ITER& ii)
             return;
         }
 
+        // r0 copy is needed only if:
+        // a. pre-emption VISA option is enabled OR
+        // b. current object is kernel with stack calls since VISA ABI requires r0 copy to be available in a pre-defined register
+        if (!R0CopyNeeded())
+            return;
+
         // Skip copying of ``copy of R0'' if it's never assigned, a case where
         // ``copy of R0'' is never used. As EOT always use ``copy of R0'', that
         // case only happens for synthetic tests where no practical code is

visa/Optimizer.h

@@ -196,6 +196,9 @@ class Optimizer
 
     void accSubPostSchedule();
 
+    // return true if BuiltInR0 gets a different allocation than r0
+    bool R0CopyNeeded();
+
 private:
     /* below member functions are used for message header opt */
     bool isHeaderOptCandidate(G4_INST *, G4_INST *);

visa/PhyRegUsage.cpp

@@ -1287,7 +1287,6 @@ void getForbiddenGRFs(
     if (kernel.getKernelType() != VISA_3D ||
         kernel.getOption(vISA_enablePreemption) ||
         reserveSpillSize > 0 ||
-        stackCallRegSize > 0 ||
         kernel.getOption(vISA_ReserveR0))
     {
         regNum.push_back(0);