Fix incorrect 2d loads rescheduling rollback functionality

admitric · pszymich · commit 5639d3ff43b3 · 2024-09-05T14:40:50.000+02:00
Fix incorrect 2d loads rescheduling rollback functionality by ensuring the uses of a potential candidate are dominated by the undo point And create an option to make splitting of the candidates finer in order to reduce the regpressure better (cherry picked from commit 8a50745)
diff --git a/IGC/Compiler/CISACodeGen/CodeSinking.cpp b/IGC/Compiler/CISACodeGen/CodeSinking.cpp
@@ -789,9 +789,9 @@ namespace IGC {
 #define LOOPSINK_PREHEADER_IMPACT_THRESHOLD 0.2
 
     // Helper functions for loop sink debug dumps
-#define PrintDump(Contents) if (IGC_IS_FLAG_ENABLED(DumpLoopSink)) {LogStream << Contents;}
-#define PrintInstructionDump(Inst) if (IGC_IS_FLAG_ENABLED(DumpLoopSink)) {(Inst)->print(LogStream, false); LogStream << "\n";}
-#define PrintOUGDump(OUG) if (IGC_IS_FLAG_ENABLED(DumpLoopSink)) {OUG.print(LogStream); LogStream << "\n";}
+#define PrintDump(Contents) if (IGC_IS_FLAG_ENABLED(DumpLoopSink)) {*LogStream << Contents;}
+#define PrintInstructionDump(Inst) if (IGC_IS_FLAG_ENABLED(DumpLoopSink)) {(Inst)->print(*LogStream, false); *LogStream << "\n";}
+#define PrintOUGDump(OUG) if (IGC_IS_FLAG_ENABLED(DumpLoopSink)) {OUG.print(*LogStream); *LogStream << "\n";}
 
 
     // Register pass to igc-opt
@@ -812,7 +812,11 @@ namespace IGC {
         IGC_INITIALIZE_PASS_END(CodeLoopSinking, PASS_FLAG1, PASS_DESCRIPTION1, PASS_CFG_ONLY1, PASS_ANALYSIS1)
 
         char CodeLoopSinking::ID = 0;
-        CodeLoopSinking::CodeLoopSinking() : FunctionPass(ID), LogStream(Log) {
+        CodeLoopSinking::CodeLoopSinking() : FunctionPass(ID), LogStringStream(Log) {
+            if (IGC_IS_FLAG_ENABLED(PrintToConsole))
+                LogStream = &IGC::Debug::ods();
+            else
+                LogStream = &LogStringStream;
             initializeCodeLoopSinkingPass(*PassRegistry::getPassRegistry());
     }
 
@@ -834,7 +838,7 @@ namespace IGC {
 
         if (IGC_IS_FLAG_ENABLED(DumpLoopSink))
         {
-            auto printGlobalSettings = [](llvm::raw_string_ostream &LogStream)
+            auto printGlobalSettings = [](llvm::raw_ostream &LogStream)
             {
                 // print every value to the dump
                 LogStream << "ForceLoopSink: " << IGC_GET_FLAG_VALUE(ForceLoopSink) << "\n";
@@ -859,7 +863,7 @@ namespace IGC {
 
             Log.clear();
 
-            printGlobalSettings(LogStream);
+            printGlobalSettings(*LogStream);
 
             PrintDump("=====================================\n");
             PrintDump("Function " << F.getName() << "\n");
@@ -900,12 +904,9 @@ namespace IGC {
             IGC_ASSERT(false == verifyFunction(F, &dbgs()));
         }
 
-        if (IGC_IS_FLAG_ENABLED(DumpLoopSink))
+        if (IGC_IS_FLAG_ENABLED(DumpLoopSink) && IGC_IS_FLAG_DISABLED(PrintToConsole))
         {
-            if (IGC_IS_FLAG_ENABLED(PrintToConsole))
-                IGC::Debug::ods() << Log;
-            else
-                dumpToFile(Log);
+            dumpToFile(Log);
         }
 
         return Changed;
@@ -1749,7 +1750,7 @@ namespace IGC {
         auto allUsesAreDominatedByRemainingUses = [&](SmallVector<Instruction *, 16> &CurrentCandidateInsts,
             SmallPtrSet<Instruction *, 16> &RemainingCandidateInsts)
         {
-            for (Instruction *RI : RemainingCandidateInsts)
+            auto instUsesDominateAllCurrentCandidateUses = [&](Instruction *RI)
             {
                 for (User *RU : RI->users())
                 {
@@ -1768,13 +1769,38 @@ namespace IGC {
                         }
                     }
                 }
+                return true;
+            };
+
+            if (IGC_IS_FLAG_ENABLED(LoopSinkCoarserLoadsRescheduling))
+                return std::all_of(RemainingCandidateInsts.begin(), RemainingCandidateInsts.end(), instUsesDominateAllCurrentCandidateUses);
+            else
+                return std::any_of(RemainingCandidateInsts.begin(), RemainingCandidateInsts.end(), instUsesDominateAllCurrentCandidateUses);
+        };
+
+        // If the uses are not dominated by the UndoPoint
+        // It's possible that we put some instructions after their uses on rollback
+        // So it needs to be checked if we sink not from PH
+        auto allUsesAreDominatedByUndoPoint = [&](SmallVector<Instruction *, 16> &CurrentCandidateInsts, Instruction *UndoPoint)
+        {
+            for (Instruction *CI : CurrentCandidateInsts)
+            {
+                for (User *CU : CI->users())
+                {
+                    Instruction *CUI = dyn_cast<Instruction>(CU);
+                    if (!CUI)
+                        return false;
+                    if (!DT->dominates(UndoPoint, CUI))
+                        return false;
+                }
             }
             return true;
         };
 
         // All the uses are a candidate
         // Try splitting then into separate candidates for better scheduling within a BB
-        auto Worthiness = I->getParent() == PH ? LoopSinkWorthiness::Sink : LoopSinkWorthiness::IntraLoopSink;
+        bool SinkFromPH = I->getParent() == PH;
+        auto Worthiness = SinkFromPH ? LoopSinkWorthiness::Sink : LoopSinkWorthiness::IntraLoopSink;
         SmallVector<Instruction *, 16> CurrentCandidateInsts;
         SmallPtrSet<Instruction *, 16> RemainingCandidateInsts(CandidateInsts.begin(), CandidateInsts.end());
 
@@ -1789,7 +1815,8 @@ namespace IGC {
 
             if (CurrentCandidateInsts.size() > 0 &&
                 Id == GenISAIntrinsic::GenISA_LSC2DBlockReadAddrPayload &&
-                allUsesAreDominatedByRemainingUses(CurrentCandidateInsts, RemainingCandidateInsts))
+                allUsesAreDominatedByRemainingUses(CurrentCandidateInsts, RemainingCandidateInsts) &&
+                (SinkFromPH || allUsesAreDominatedByUndoPoint(CurrentCandidateInsts, CurrentCandidateInsts[0]->getNextNode())))
             {
                 NCandidates++;
                 SinkCandidates.push_back(std::make_unique<Candidate>(CurrentCandidateInsts, TgtBB, Worthiness, CurrentCandidateInsts[0]->getNextNode()));
@@ -1798,7 +1825,8 @@ namespace IGC {
             CurrentCandidateInsts.push_back(I);
             RemainingCandidateInsts.erase(I);
         }
-        if (CurrentCandidateInsts.size() > 0)
+        if (CurrentCandidateInsts.size() > 0 &&
+            (SinkFromPH || allUsesAreDominatedByUndoPoint(CurrentCandidateInsts, CurrentCandidateInsts[0]->getNextNode())))
         {
             NCandidates++;
             SinkCandidates.push_back(std::make_unique<Candidate>(CurrentCandidateInsts, TgtBB, Worthiness, CurrentCandidateInsts[0]->getNextNode()));
diff --git a/IGC/Compiler/CISACodeGen/CodeSinking.hpp b/IGC/Compiler/CISACodeGen/CodeSinking.hpp
@@ -239,7 +239,8 @@ namespace IGC {
         llvm::SmallPtrSet<llvm::BasicBlock*, 8> UndoBlkSet;
         /// dumping
         std::string Log;
-        llvm::raw_string_ostream LogStream;
+        llvm::raw_string_ostream LogStringStream;
+        llvm::raw_ostream *LogStream = nullptr;
 
         void dumpToFile(const std::string& Log);
 
diff --git a/IGC/Compiler/tests/CodeSinking/LoopSinking/2d-blockload-loopsched.ll b/IGC/Compiler/tests/CodeSinking/LoopSinking/2d-blockload-loopsched.ll
@@ -6,7 +6,7 @@
 ;
 ;============================ end_copyright_notice =============================
 ; REQUIRES: regkeys, llvm-14-plus
-; RUN: igc_opt --regkey LoopSinkMinSave=0 --regkey LoopSinkEnable2dBlockReads=1 --regkey LoopSinkEnableLoadsRescheduling=1 --regkey ForceLoopSink=1 --regkey CodeLoopSinkingMinSize=10 --regkey LoopSinkDisableRollback=1 --regkey CodeSinkingLoadSchedulingInstr=0 %enable-basic-aa% --igc-code-loop-sinking -S %s 2>&1 | FileCheck %s
+; RUN: igc_opt --regkey LoopSinkMinSave=0 --regkey LoopSinkEnable2dBlockReads=1 --regkey LoopSinkCoarserLoadsRescheduling=1 --regkey LoopSinkEnableLoadsRescheduling=1 --regkey ForceLoopSink=1 --regkey CodeLoopSinkingMinSize=10 --regkey LoopSinkDisableRollback=1 --regkey CodeSinkingLoadSchedulingInstr=0 %enable-basic-aa% --igc-code-loop-sinking -S %s 2>&1 | FileCheck %s
 
 declare i8* @llvm.genx.GenISA.LSC2DBlockCreateAddrPayload.p0i8(i64, i32, i32, i32, i32, i32, i32, i32, i32) #0
 
diff --git a/IGC/Compiler/tests/CodeSinking/LoopSinking/2d-blockload-sch-rollback.ll b/IGC/Compiler/tests/CodeSinking/LoopSinking/2d-blockload-sch-rollback.ll
@@ -108,6 +108,44 @@ for.body19.i:                                     ; preds = %for.body19.i.for.bo
   br label %for.body19.i
 }
 
+
+; If all block reads are considered a single candidate and we don't check the undo point is dominated
+; We might generate incorrect IR by placing the first block read after the first dpas
+; Checking it's not happening
+
+define spir_kernel void @bar() {
+; CHECK-LABEL: @bar(
+; CHECK:         br label [[DOT_CRIT_EDGE:%.*]]
+; CHECK:       ._crit_edge:
+; CHECK:         [[BLOCK2D_ADDRPAYLOAD456:%.*]] = call i32* @llvm.genx.GenISA.LSC2DBlockCreateAddrPayload.p0i32(i64 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0)
+; CHECK:         [[BLOCK2D_READADDRPAYLOAD457:%.*]] = call <8 x i32> @llvm.genx.GenISA.LSC2DBlockReadAddrPayload.v8i32.p0i32(i32* [[BLOCK2D_ADDRPAYLOAD456]], i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i1 false, i1 false, i32 0)
+; CHECK:         [[TMP1:%.*]] = call <8 x float> @llvm.genx.GenISA.sub.group.dpas.v8f32.v8f32.v8i16.v8i32(<8 x float> zeroinitializer, <8 x i16> zeroinitializer, <8 x i32> [[BLOCK2D_READADDRPAYLOAD457]], i32 0, i32 0, i32 0, i32 0, i1 false)
+; CHECK:         [[TMP2:%.*]] = insertelement <8 x half> zeroinitializer, half 0xH0000, i64 0
+; CHECK:         [[TMP3:%.*]] = insertelement <8 x half> [[TMP2]], half 0xH0000, i64 0
+; CHECK:         [[BLOCK2D_READADDRPAYLOAD459:%.*]] = call <8 x i32> @llvm.genx.GenISA.LSC2DBlockReadAddrPayload.v8i32.p0i32(i32* [[BLOCK2D_ADDRPAYLOAD456]], i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i1 false, i1 false, i32 0)
+; CHECK:         [[TMP4:%.*]] = call <8 x float> @llvm.genx.GenISA.sub.group.dpas.v8f32.v8f32.v8i16.v8i32(<8 x float> zeroinitializer, <8 x i16> zeroinitializer, <8 x i32> [[BLOCK2D_READADDRPAYLOAD459]], i32 0, i32 0, i32 0, i32 0, i1 false)
+; CHECK:         [[TMP5:%.*]] = call <8 x float> @llvm.genx.GenISA.sub.group.dpas.v8f32.v8f32.v8i16.v8i32(<8 x float> zeroinitializer, <8 x i16> zeroinitializer, <8 x i32> [[BLOCK2D_READADDRPAYLOAD457]], i32 0, i32 0, i32 0, i32 0, i1 false)
+; CHECK:         br label [[DOT_CRIT_EDGE___CRIT_EDGE_CRIT_EDGE:%.*]]
+; CHECK:       ._crit_edge.._crit_edge_crit_edge:
+; CHECK:         br label [[DOT_CRIT_EDGE]]
+;
+  br label %._crit_edge
+
+._crit_edge:                                      ; preds = %._crit_edge.._crit_edge_crit_edge, %0
+  %Block2D_AddrPayload456 = call i32* @llvm.genx.GenISA.LSC2DBlockCreateAddrPayload.p0i32(i64 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0)
+  %Block2D_ReadAddrPayload457 = call <8 x i32> @llvm.genx.GenISA.LSC2DBlockReadAddrPayload.v8i32.p0i32(i32* %Block2D_AddrPayload456, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i1 false, i1 false, i32 0)
+  %1 = call <8 x float> @llvm.genx.GenISA.sub.group.dpas.v8f32.v8f32.v8i16.v8i32(<8 x float> zeroinitializer, <8 x i16> zeroinitializer, <8 x i32> %Block2D_ReadAddrPayload457, i32 0, i32 0, i32 0, i32 0, i1 false)
+  %2 = insertelement <8 x half> zeroinitializer, half 0xH0000, i64 0
+  %3 = insertelement <8 x half> %2, half 0xH0000, i64 0
+  %Block2D_ReadAddrPayload459 = call <8 x i32> @llvm.genx.GenISA.LSC2DBlockReadAddrPayload.v8i32.p0i32(i32* %Block2D_AddrPayload456, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i1 false, i1 false, i32 0)
+  %4 = call <8 x float> @llvm.genx.GenISA.sub.group.dpas.v8f32.v8f32.v8i16.v8i32(<8 x float> zeroinitializer, <8 x i16> zeroinitializer, <8 x i32> %Block2D_ReadAddrPayload459, i32 0, i32 0, i32 0, i32 0, i1 false)
+  %5 = call <8 x float> @llvm.genx.GenISA.sub.group.dpas.v8f32.v8f32.v8i16.v8i32(<8 x float> zeroinitializer, <8 x i16> zeroinitializer, <8 x i32> %Block2D_ReadAddrPayload457, i32 0, i32 0, i32 0, i32 0, i1 false)
+  br label %._crit_edge.._crit_edge_crit_edge
+
+._crit_edge.._crit_edge_crit_edge:                ; preds = %._crit_edge
+  br label %._crit_edge
+}
+
 declare <8 x float> @llvm.genx.GenISA.sub.group.dpas.v8f32.v8f32.v8i16.v8i32(<8 x float>, <8 x i16>, <8 x i32>, i32, i32, i32, i32, i1)
 
 declare i32* @llvm.genx.GenISA.LSC2DBlockCreateAddrPayload.p0i32(i64, i32, i32, i32, i32, i32, i32, i32, i32)
diff --git a/IGC/common/igc_flags.h b/IGC/common/igc_flags.h
@@ -152,6 +152,7 @@ DECLARE_IGC_REGKEY(DWORD, LoopSinkMinSave,              1,  "If loop sink can ha
 DECLARE_IGC_REGKEY(DWORD, LoopSinkThresholdDelta,       30,  "Do loop sink If the estimated register pressure is higher than this + #avaialble registers", false)
 DECLARE_IGC_REGKEY(DWORD, LoopSinkRollbackThreshold,    15,  "Rollback loop sinking if the estimated regpressure after the sinking is still higher than this + #available registers, and the number of registers can be increased", false)
 DECLARE_IGC_REGKEY(bool, LoopSinkEnableLoadsRescheduling, true, "Allow sinking the loads that are already in the loop", false)
+DECLARE_IGC_REGKEY(bool, LoopSinkCoarserLoadsRescheduling, true, "Try to reschedule multi-instruction load candidates in larger chunks", false)
 DECLARE_IGC_REGKEY(bool, LoopSinkEnable2dBlockReads,    true,  "Allow sinking of the 2d block reads", false)
 DECLARE_IGC_REGKEY(bool, LoopSinkEnableVectorShuffle,   true,  "Allow sinking of the lowered vector shuffle pattern", false)
 DECLARE_IGC_REGKEY(bool, LoopSinkForceRollback,         false,  "Rollback every loop sinking change (for debug purposes only)", false)

Original file line number	Diff line number	Diff line change
`@@ -6,7 +6,7 @@`
`6`	`6`	`;`
`7`	`7`	`;============================ end_copyright_notice =============================`
`8`	`8`	`; REQUIRES: regkeys, llvm-14-plus`
`9`		`-; RUN: igc_opt --regkey LoopSinkMinSave=0 --regkey LoopSinkEnable2dBlockReads=1 --regkey LoopSinkEnableLoadsRescheduling=1 --regkey ForceLoopSink=1 --regkey CodeLoopSinkingMinSize=10 --regkey LoopSinkDisableRollback=1 --regkey CodeSinkingLoadSchedulingInstr=0 %enable-basic-aa% --igc-code-loop-sinking -S %s 2>&1 \| FileCheck %s`
	`9`	`+; RUN: igc_opt --regkey LoopSinkMinSave=0 --regkey LoopSinkEnable2dBlockReads=1 --regkey LoopSinkCoarserLoadsRescheduling=1 --regkey LoopSinkEnableLoadsRescheduling=1 --regkey ForceLoopSink=1 --regkey CodeLoopSinkingMinSize=10 --regkey LoopSinkDisableRollback=1 --regkey CodeSinkingLoadSchedulingInstr=0 %enable-basic-aa% --igc-code-loop-sinking -S %s 2>&1 \| FileCheck %s`
`10`	`10`
`11`	`11`	`declare i8* @llvm.genx.GenISA.LSC2DBlockCreateAddrPayload.p0i8(i64, i32, i32, i32, i32, i32, i32, i32, i32) #0`
`12`	`12`