From f47aaf8c03c8ba6a0ed9d2f61582dd5ab7bb23fa Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Mon, 15 Jan 2024 17:38:38 -0500 Subject: [PATCH 1/3] Allow jump removal for aligned BBJ_ALWAYS --- src/coreclr/jit/block.cpp | 2 +- src/coreclr/jit/codegenlinear.cpp | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/block.cpp b/src/coreclr/jit/block.cpp index c365b2a774a07..318e889969d54 100644 --- a/src/coreclr/jit/block.cpp +++ b/src/coreclr/jit/block.cpp @@ -297,7 +297,7 @@ bool BasicBlock::IsFirstColdBlock(Compiler* compiler) const bool BasicBlock::CanRemoveJumpToNext(Compiler* compiler) const { assert(KindIs(BBJ_ALWAYS)); - return JumpsToNext() && !hasAlign() && !compiler->fgInDifferentRegions(this, bbTarget); + return JumpsToNext() && (bbNext != compiler->fgFirstColdBlock); } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index 83712ec0cd669..3eaa6a7e7ca41 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -645,6 +645,7 @@ void CodeGen::genCodeForBBlist() /* Do we need to generate a jump or return? */ + bool removedJmp = false; switch (block->GetKind()) { case BBJ_RETURN: @@ -733,6 +734,7 @@ void CodeGen::genCodeForBBlist() } #endif // TARGET_AMD64 + removedJmp = true; break; } #ifdef TARGET_XARCH @@ -811,7 +813,7 @@ void CodeGen::genCodeForBBlist() assert(!block->KindIs(BBJ_CALLFINALLY)); #endif // FEATURE_EH_CALLFINALLY_THUNKS - GetEmitter()->emitLoopAlignment(DEBUG_ARG1(block->KindIs(BBJ_ALWAYS))); + GetEmitter()->emitLoopAlignment(DEBUG_ARG1(block->KindIs(BBJ_ALWAYS) && !removedJmp)); } if (!block->IsLast() && block->Next()->isLoopAlign()) From 188f2b22e68461597052d63a1af7fb2ce71c2927 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Wed, 17 Jan 2024 10:35:51 -0500 Subject: [PATCH 2/3] no merge --- src/coreclr/jit/block.cpp | 2 +- src/coreclr/jit/block.h | 1 + src/coreclr/jit/compiler.cpp | 7 +++++++ src/coreclr/scripts/superpmi.py | 2 +- 4 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/block.cpp b/src/coreclr/jit/block.cpp index 318e889969d54..ac0de741f8650 100644 --- a/src/coreclr/jit/block.cpp +++ b/src/coreclr/jit/block.cpp @@ -297,7 +297,7 @@ bool BasicBlock::IsFirstColdBlock(Compiler* compiler) const bool BasicBlock::CanRemoveJumpToNext(Compiler* compiler) const { assert(KindIs(BBJ_ALWAYS)); - return JumpsToNext() && (bbNext != compiler->fgFirstColdBlock); + return JumpsToNext() && (bbNext != compiler->fgFirstColdBlock) && (!hasAlign() || HasFlag(BBF_JMP_TO_NESTED_LOOP)); } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/block.h b/src/coreclr/jit/block.h index ba8156be7a35d..723defc100212 100644 --- a/src/coreclr/jit/block.h +++ b/src/coreclr/jit/block.h @@ -429,6 +429,7 @@ enum BasicBlockFlags : unsigned __int64 // (TODO: Remove this quirk after refactoring Compiler::fgFindInsertPoint) BBF_OLD_LOOP_HEADER_QUIRK = MAKE_BBFLAG(42), // Block was the header ('entry') of a loop recognized by old loop finding BBF_HAS_VALUE_PROFILE = MAKE_BBFLAG(43), // Block has a node that needs a value probing + BBF_JMP_TO_NESTED_LOOP = MAKE_BBFLAG(44), // The following are sets of flags. diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index a69527436913a..65ccdaafd2b2e 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -5431,6 +5431,13 @@ PhaseStatus Compiler::placeLoopAlignInstructions() bbHavingAlign = prev; JITDUMP("Marking " FMT_BB " before the loop with BBF_HAS_ALIGN for loop at " FMT_BB "\n", prev->bbNum, block->bbNum); + + // bbHavingAlign is in a loop, and precedes a nested loop + if (blockToLoop->GetLoop(bbHavingAlign) != nullptr) + { + // If bbHavingAlign is a removable jump, it will be removed despite it having alignment + bbHavingAlign->SetFlags(BBF_JMP_TO_NESTED_LOOP); + } } else { diff --git a/src/coreclr/scripts/superpmi.py b/src/coreclr/scripts/superpmi.py index 23bd265620dfd..aa013d768f3df 100644 --- a/src/coreclr/scripts/superpmi.py +++ b/src/coreclr/scripts/superpmi.py @@ -1971,7 +1971,7 @@ def replay_with_asm_diffs(self): # These vars are force overridden in the SPMI runs for both the base and diff, always. replay_vars = { - "DOTNET_JitAlignLoops": "0", # disable loop alignment to filter noise + "DOTNET_JitAlignLoops": "1", # disable loop alignment to filter noise "DOTNET_JitEnableNoWayAssert": "1", "DOTNET_JitNoForceFallback": "1", } From b1a138f3cc60c2fda1dcaa0ac9d90cd65e9a01d9 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 2 Feb 2024 10:32:22 -0500 Subject: [PATCH 3/3] Revert "no merge" This reverts commit 188f2b22e68461597052d63a1af7fb2ce71c2927. --- src/coreclr/jit/block.cpp | 2 +- src/coreclr/jit/block.h | 1 - src/coreclr/jit/compiler.cpp | 7 ------- src/coreclr/scripts/superpmi.py | 2 +- 4 files changed, 2 insertions(+), 10 deletions(-) diff --git a/src/coreclr/jit/block.cpp b/src/coreclr/jit/block.cpp index ac0de741f8650..318e889969d54 100644 --- a/src/coreclr/jit/block.cpp +++ b/src/coreclr/jit/block.cpp @@ -297,7 +297,7 @@ bool BasicBlock::IsFirstColdBlock(Compiler* compiler) const bool BasicBlock::CanRemoveJumpToNext(Compiler* compiler) const { assert(KindIs(BBJ_ALWAYS)); - return JumpsToNext() && (bbNext != compiler->fgFirstColdBlock) && (!hasAlign() || HasFlag(BBF_JMP_TO_NESTED_LOOP)); + return JumpsToNext() && (bbNext != compiler->fgFirstColdBlock); } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/block.h b/src/coreclr/jit/block.h index 723defc100212..ba8156be7a35d 100644 --- a/src/coreclr/jit/block.h +++ b/src/coreclr/jit/block.h @@ -429,7 +429,6 @@ enum BasicBlockFlags : unsigned __int64 // (TODO: Remove this quirk after refactoring Compiler::fgFindInsertPoint) BBF_OLD_LOOP_HEADER_QUIRK = MAKE_BBFLAG(42), // Block was the header ('entry') of a loop recognized by old loop finding BBF_HAS_VALUE_PROFILE = MAKE_BBFLAG(43), // Block has a node that needs a value probing - BBF_JMP_TO_NESTED_LOOP = MAKE_BBFLAG(44), // The following are sets of flags. diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 65ccdaafd2b2e..a69527436913a 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -5431,13 +5431,6 @@ PhaseStatus Compiler::placeLoopAlignInstructions() bbHavingAlign = prev; JITDUMP("Marking " FMT_BB " before the loop with BBF_HAS_ALIGN for loop at " FMT_BB "\n", prev->bbNum, block->bbNum); - - // bbHavingAlign is in a loop, and precedes a nested loop - if (blockToLoop->GetLoop(bbHavingAlign) != nullptr) - { - // If bbHavingAlign is a removable jump, it will be removed despite it having alignment - bbHavingAlign->SetFlags(BBF_JMP_TO_NESTED_LOOP); - } } else { diff --git a/src/coreclr/scripts/superpmi.py b/src/coreclr/scripts/superpmi.py index aa013d768f3df..23bd265620dfd 100644 --- a/src/coreclr/scripts/superpmi.py +++ b/src/coreclr/scripts/superpmi.py @@ -1971,7 +1971,7 @@ def replay_with_asm_diffs(self): # These vars are force overridden in the SPMI runs for both the base and diff, always. replay_vars = { - "DOTNET_JitAlignLoops": "1", # disable loop alignment to filter noise + "DOTNET_JitAlignLoops": "0", # disable loop alignment to filter noise "DOTNET_JitEnableNoWayAssert": "1", "DOTNET_JitNoForceFallback": "1", }