From df256df6dcb9645cdc8433b5cffa63e7ed18afbe Mon Sep 17 00:00:00 2001 From: Pete Chou Date: Mon, 2 Oct 2023 23:28:40 -0700 Subject: [PATCH] [Autobackout][FuncReg]Revert of change: 502f5552ed3345d07abb56bc1369a2deafea72f0 Reland the WA that emits a NOP after branch instruction to support single stepping in debugger. The WA is enabled only when compiling with -O0 currently. --- IGC/Compiler/CISACodeGen/CISABuilder.cpp | 6 ----- IGC/Compiler/CISACodeGen/Platform.hpp | 13 ---------- visa/Optimizer.h | 1 - visa/SWWA.cpp | 33 ------------------------ visa/include/VISAOptionsDefs.h | 2 -- 5 files changed, 55 deletions(-) diff --git a/IGC/Compiler/CISACodeGen/CISABuilder.cpp b/IGC/Compiler/CISACodeGen/CISABuilder.cpp index e21cf55c06e8..183e2b202119 100644 --- a/IGC/Compiler/CISACodeGen/CISABuilder.cpp +++ b/IGC/Compiler/CISACodeGen/CISABuilder.cpp @@ -5105,12 +5105,6 @@ namespace IGC if (IGC_IS_FLAG_ENABLED(deadLoopForFloatException)) { SaveOption(vISA_AddIEEEExceptionTrap, true); } - - // WA to support single stepping in debugger. Currently only enabled it - // when compiling with -O0. - if (isOptDisabled && m_program->m_Platform->WaAddNopAfterBranchInst()) { - SaveOption(vISA_GenerateNopAfterCFInst, true); - } } // InitVISABuilderOptions // Get a unqiue label for inline asm instruction blocks at the module level. diff --git a/IGC/Compiler/CISACodeGen/Platform.hpp b/IGC/Compiler/CISACodeGen/Platform.hpp index 33c97dcdff9e..03b031a45160 100644 --- a/IGC/Compiler/CISACodeGen/Platform.hpp +++ b/IGC/Compiler/CISACodeGen/Platform.hpp @@ -1494,18 +1494,5 @@ unsigned int roundUpTgsmSize(DWORD size) const return iSTD::RoundPower2(size) * blockSize; } -bool WaAddNopAfterBranchInst() const -{ - if (m_platformInfo.eProductFamily == IGFX_PVC) - return true; - - if (m_platformInfo.eProductFamily == IGFX_DG2 && - (GFX_IS_DG2_G11_CONFIG(m_platformInfo.usDeviceID) || - GFX_IS_DG2_G12_CONFIG(m_platformInfo.usDeviceID))) - return true; - - return false; -} - }; }//namespace IGC diff --git a/visa/Optimizer.h b/visa/Optimizer.h index f6debe98714d..2a09ccc35b68 100644 --- a/visa/Optimizer.h +++ b/visa/Optimizer.h @@ -260,7 +260,6 @@ class Optimizer { void applyNamedBarrierWA(INST_LIST_ITER it, G4_BB *bb); void insertIEEEExceptionTrap(); void expandIEEEExceptionTrap(INST_LIST_ITER it, G4_BB *bb); - void insertNopAfterCFInst(BB_LIST_ITER ib, INST_LIST_ITER it); typedef std::vector InstListType; // create instruction sequence to calculate call offset from ip diff --git a/visa/SWWA.cpp b/visa/SWWA.cpp index d3ad11982d19..3910d36e86c6 100644 --- a/visa/SWWA.cpp +++ b/visa/SWWA.cpp @@ -3337,36 +3337,6 @@ void Optimizer::expandIEEEExceptionTrap(INST_LIST_ITER it, G4_BB *bb) { *it = restoreFlag; } -void Optimizer::insertNopAfterCFInst(BB_LIST_ITER ib, INST_LIST_ITER ii) { - // If the CF inst is not the last inst of current BB, simply insert - // a nop after it. Otherwise, insert a dummy BB to hold a nop between - // the current BB and the fall-through BB. - G4_BB *bb = *ib; - G4_INST *inst = *ii; - if (inst != bb->back()) { - bb->insertAfter(ii, builder.createNop(InstOpt_NoOpt)); - } else { - G4_BB *dummyBB = fg.createNewBBWithLabel("CFInstWA_BB"); - dummyBB->push_back(builder.createNop(InstOpt_NoOpt)); - BB_LIST_ITER nextBI = std::next(ib); - fg.insert(nextBI, dummyBB); - // Update the pred/succ edges accordingly. - bb->setPhysicalSucc(dummyBB); - dummyBB->setPhysicalPred(bb); - if (nextBI != fg.end()) { - G4_BB *nextBB = *nextBI; - dummyBB->setPhysicalSucc(nextBB); - nextBB->setPhysicalPred(dummyBB); - if (std::any_of(bb->Succs.begin(), bb->Succs.end(), - [=](G4_BB *succ) { return succ == nextBB; } )) { - fg.removePredSuccEdges(bb, nextBB); - fg.addPredSuccEdges(bb, dummyBB); - fg.addPredSuccEdges(dummyBB, nextBB); - } - } - } -} - // For a subroutine, insert a dummy move with {Switch} option immediately // before the first non-label instruction in BB. Otherwie, for a following // basic block, insert a dummy move before *any* instruction to ensure that @@ -3797,9 +3767,6 @@ void Optimizer::HWWorkaround() { if (inst->isIEEEExceptionTrap()) expandIEEEExceptionTrap(ii, bb); - if (inst->isCFInst() && builder.getOption(vISA_GenerateNopAfterCFInst)) - insertNopAfterCFInst(ib, ii); - ii++; } } diff --git a/visa/include/VISAOptionsDefs.h b/visa/include/VISAOptionsDefs.h index 1ca536ebb67a..96ab5aba7d57 100644 --- a/visa/include/VISAOptionsDefs.h +++ b/visa/include/VISAOptionsDefs.h @@ -641,8 +641,6 @@ DEF_VISA_OPTION(vISA_noIndirectSrcForCompressedInstWA, ET_BOOL, DEF_VISA_OPTION(vISA_GenerateDebugInfo, ET_BOOL, "-generateDebugInfo", UNUSED, false) DEF_VISA_OPTION(vISA_setStartBreakPoint, ET_BOOL, "-setstartbp", UNUSED, false) -DEF_VISA_OPTION(vISA_GenerateNopAfterCFInst, ET_BOOL, - "-generateNopAfterCFInst", UNUSED, false) DEF_VISA_OPTION(vISA_InsertHashMovs, ET_BOOL, NULLSTR, UNUSED, false) DEF_VISA_OPTION(vISA_InsertDummyMovForHWRSWA, ET_BOOL, "-insertRSDummyMov", UNUSED, false)