From 2264fb6e2033c78cc89e8087e925def8ad7a7cb6 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Sun, 10 Sep 2023 05:10:55 -0700 Subject: [PATCH 1/3] Check if loop body occured before loopTop" --- src/coreclr/jit/compiler.cpp | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index a04bd56057abb..95ce1bfe03bb7 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -5282,6 +5282,13 @@ PhaseStatus Compiler::placeLoopAlignInstructions() weight_t minBlockSoFar = BB_MAX_WEIGHT; BasicBlock* bbHavingAlign = nullptr; BasicBlock::loopNumber currentAlignedLoopNum = BasicBlock::NOT_IN_LOOP; + bool visitedLoopNum[BasicBlock::MAX_LOOP_NUM]; + memset(visitedLoopNum, false, sizeof(visitedLoopNum)); + +#ifdef DEBUG + int visitedBlockForLoopNum[BasicBlock::MAX_LOOP_NUM]; + memset(visitedBlockForLoopNum, false, sizeof(visitedBlockForLoopNum)); +#endif if ((fgFirstBB != nullptr) && fgFirstBB->isLoopAlign()) { @@ -5304,7 +5311,7 @@ PhaseStatus Compiler::placeLoopAlignInstructions() } } - // If there is a unconditional jump (which is not part of callf/always pair) + // If there is an unconditional jump (which is not part of callf/always pair) if (opts.compJitHideAlignBehindJmp && (block->bbJumpKind == BBJ_ALWAYS) && !block->isBBCallAlwaysPairTail()) { // Track the lower weight blocks @@ -5358,12 +5365,19 @@ PhaseStatus Compiler::placeLoopAlignInstructions() madeChanges = true; unmarkedLoopAlign = true; } - else if ((block->bbNatLoopNum != BasicBlock::NOT_IN_LOOP) && (block->bbNatLoopNum == loopTop->bbNatLoopNum)) + else if (visitedLoopNum[loopTop->bbNatLoopNum]) { +#ifdef DEBUG + char buffer[50]; + sprintf_s(buffer, 50, "loop block " FMT_BB " appears before top of loop", + visitedBlockForLoopNum[loopTop->bbNatLoopNum]); +#endif + // In some odd cases we may see blocks within the loop before we see the // top block of the loop. Just bail on aligning such loops. // - loopTop->unmarkLoopAlign(this DEBUG_ARG("loop block appears before top of loop")); + + loopTop->unmarkLoopAlign(this DEBUG_ARG(buffer)); madeChanges = true; unmarkedLoopAlign = true; } @@ -5398,6 +5412,20 @@ PhaseStatus Compiler::placeLoopAlignInstructions() break; } } + + if (block->bbNatLoopNum != BasicBlock::NOT_IN_LOOP) + { +#ifdef DEBUG + if (!visitedLoopNum[block->bbNatLoopNum]) + { + // Record the first block for which bbNatLoopNum was seen for + // debugging purpose. + visitedBlockForLoopNum[block->bbNatLoopNum] = block->bbNum; + } +#endif + // If this block is part of loop, mark the loopNum as visited. + visitedLoopNum[block->bbNatLoopNum] = true; + } } assert(loopsToProcess == 0); From db15d803d620f8baeaec2f73e0eb27a58565956b Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Fri, 15 Sep 2023 09:15:58 -0700 Subject: [PATCH 2/3] Check if bbNatLoopNum is not NOT_IN_LOOP --- src/coreclr/jit/compiler.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 95ce1bfe03bb7..00fc26062f5d6 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -5286,8 +5286,8 @@ PhaseStatus Compiler::placeLoopAlignInstructions() memset(visitedLoopNum, false, sizeof(visitedLoopNum)); #ifdef DEBUG - int visitedBlockForLoopNum[BasicBlock::MAX_LOOP_NUM]; - memset(visitedBlockForLoopNum, false, sizeof(visitedBlockForLoopNum)); + unsigned visitedBlockForLoopNum[BasicBlock::MAX_LOOP_NUM]; + memset(visitedBlockForLoopNum, 0, sizeof(visitedBlockForLoopNum)); #endif if ((fgFirstBB != nullptr) && fgFirstBB->isLoopAlign()) @@ -5365,11 +5365,11 @@ PhaseStatus Compiler::placeLoopAlignInstructions() madeChanges = true; unmarkedLoopAlign = true; } - else if (visitedLoopNum[loopTop->bbNatLoopNum]) + else if ((loopTop->bbNatLoopNum != BasicBlock::NOT_IN_LOOP) && (visitedLoopNum[loopTop->bbNatLoopNum])) { #ifdef DEBUG - char buffer[50]; - sprintf_s(buffer, 50, "loop block " FMT_BB " appears before top of loop", + char buffer[100]; + sprintf_s(buffer, 80, "loop block " FMT_BB " appears before top of loop", visitedBlockForLoopNum[loopTop->bbNatLoopNum]); #endif From 798f43874481299482744673a9a251ee343ad5ea Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Fri, 15 Sep 2023 10:07:45 -0700 Subject: [PATCH 3/3] review feedback --- src/coreclr/jit/compiler.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 00fc26062f5d6..d9e43e57b1d65 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -5365,11 +5365,11 @@ PhaseStatus Compiler::placeLoopAlignInstructions() madeChanges = true; unmarkedLoopAlign = true; } - else if ((loopTop->bbNatLoopNum != BasicBlock::NOT_IN_LOOP) && (visitedLoopNum[loopTop->bbNatLoopNum])) + else if ((loopTop->bbNatLoopNum != BasicBlock::NOT_IN_LOOP) && visitedLoopNum[loopTop->bbNatLoopNum]) { #ifdef DEBUG char buffer[100]; - sprintf_s(buffer, 80, "loop block " FMT_BB " appears before top of loop", + sprintf_s(buffer, 100, "loop block " FMT_BB " appears before top of loop", visitedBlockForLoopNum[loopTop->bbNatLoopNum]); #endif