From b65b96656646df5f0173201dd93f0f2486442fa8 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Mon, 29 Jan 2024 19:49:36 -0500 Subject: [PATCH 1/3] Improve BBJ_COND to BBJ_ALWAYS conversion --- src/coreclr/jit/codegencommon.cpp | 2 +- src/coreclr/jit/fgopt.cpp | 46 +++++++++++++++++++------------ 2 files changed, 30 insertions(+), 18 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 0537b050219a52..fc7998d891b00e 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -2036,7 +2036,7 @@ void CodeGen::genEmitMachineCode() printf("\n"); } - printf("Total bytes of code %d, prolog size %d, PerfScore %.2f, instruction count %d, allocated bytes for " + printf("; Total bytes of code %d, prolog size %d, PerfScore %.2f, instruction count %d, allocated bytes for " "code %d", codeSize, prologSize, compiler->info.compPerfScore, instrCount, GetEmitter()->emitTotalHotCodeSize + GetEmitter()->emitTotalColdCodeSize); diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index 52f969611a0a76..c4f9dfbbcb94b8 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -4745,7 +4745,6 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */, bool isPh BasicBlock* bPrev = nullptr; // the previous non-worthless block BasicBlock* bNext; // the successor of the current block BasicBlock* bDest; // the jump target of the current block - BasicBlock* bFalseDest; // the false target of the current block (if it is a BBJ_COND) for (block = fgFirstBB; block != nullptr; block = block->Next()) { @@ -4779,9 +4778,8 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */, bool isPh REPEAT:; - bNext = block->Next(); - bDest = nullptr; - bFalseDest = nullptr; + bNext = block->Next(); + bDest = nullptr; if (block->KindIs(BBJ_ALWAYS)) { @@ -4816,15 +4814,29 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */, bool isPh } else if (block->KindIs(BBJ_COND)) { - bDest = block->GetTrueTarget(); - bFalseDest = block->GetFalseTarget(); - if (bDest == bFalseDest) + bDest = block->GetTrueTarget(); + BasicBlock* bFalseDest = block->GetFalseTarget(); + + if (bFalseDest->KindIs(BBJ_ALWAYS) && bFalseDest->TargetIs(bDest) && bFalseDest->isEmpty()) + { + // Optimize block -> bFalseDest -> bDest into a BBJ_ALWAYS + block->SetFalseTarget(bDest); + fgAddRefPred(bDest, block, fgRemoveRefPred(bFalseDest, block)); + } + else if (bDest->KindIs(BBJ_ALWAYS) && bDest->TargetIs(bFalseDest) && bDest->isEmpty()) + { + // Optimize block -> bDest -> bFalseDest into a BBJ_ALWAYS + block->SetTrueTarget(bFalseDest); + fgAddRefPred(bFalseDest, block, fgRemoveRefPred(bDest, block)); + bDest = bFalseDest; + } + + if (block->FalseTargetIs(bDest)) { fgRemoveConditionalJump(block); assert(block->KindIs(BBJ_ALWAYS)); - change = true; - modified = true; - bFalseDest = nullptr; + change = true; + modified = true; } } @@ -4852,13 +4864,13 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */, bool isPh // (b) block jump target is elsewhere but join free, and // bNext's jump target has a join. // - if (block->KindIs(BBJ_COND) && // block is a BBJ_COND block - (bFalseDest == bNext) && // false target is the next block - (bNext->bbRefs == 1) && // no other block jumps to bNext - bNext->KindIs(BBJ_ALWAYS) && // the next block is a BBJ_ALWAYS block - !bNext->JumpsToNext() && // and it doesn't jump to the next block (we might compact them) - bNext->isEmpty() && // and it is an empty block - !bNext->TargetIs(bNext) && // special case for self jumps + if (block->KindIs(BBJ_COND) && // block is a BBJ_COND block + block->FalseTargetIs(bNext) && // false target is the next block + (bNext->bbRefs == 1) && // no other block jumps to bNext + bNext->KindIs(BBJ_ALWAYS) && // the next block is a BBJ_ALWAYS block + !bNext->JumpsToNext() && // and it doesn't jump to the next block (we might compact them) + bNext->isEmpty() && // and it is an empty block + !bNext->TargetIs(bNext) && // special case for self jumps !bDest->IsFirstColdBlock(this) && !fgInDifferentRegions(block, bDest)) // do not cross hot/cold sections { From 9f3ac60c7d7dfdb3fd4640b636035e505c9767e6 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Mon, 29 Jan 2024 19:51:39 -0500 Subject: [PATCH 2/3] Remove unrelated change --- src/coreclr/jit/codegencommon.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index fc7998d891b00e..0537b050219a52 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -2036,7 +2036,7 @@ void CodeGen::genEmitMachineCode() printf("\n"); } - printf("; Total bytes of code %d, prolog size %d, PerfScore %.2f, instruction count %d, allocated bytes for " + printf("Total bytes of code %d, prolog size %d, PerfScore %.2f, instruction count %d, allocated bytes for " "code %d", codeSize, prologSize, compiler->info.compPerfScore, instrCount, GetEmitter()->emitTotalHotCodeSize + GetEmitter()->emitTotalColdCodeSize); From cc8499c5a6116c9546ce94c9dfffc2bf000c5019 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Mon, 29 Jan 2024 19:58:02 -0500 Subject: [PATCH 3/3] Revert some small changes --- src/coreclr/jit/fgopt.cpp | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index c4f9dfbbcb94b8..367e5df0d2ccdd 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -4745,6 +4745,7 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */, bool isPh BasicBlock* bPrev = nullptr; // the previous non-worthless block BasicBlock* bNext; // the successor of the current block BasicBlock* bDest; // the jump target of the current block + BasicBlock* bFalseDest; // the false target of the current block (if it is a BBJ_COND) for (block = fgFirstBB; block != nullptr; block = block->Next()) { @@ -4778,8 +4779,9 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */, bool isPh REPEAT:; - bNext = block->Next(); - bDest = nullptr; + bNext = block->Next(); + bDest = nullptr; + bFalseDest = nullptr; if (block->KindIs(BBJ_ALWAYS)) { @@ -4814,20 +4816,24 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */, bool isPh } else if (block->KindIs(BBJ_COND)) { - bDest = block->GetTrueTarget(); - BasicBlock* bFalseDest = block->GetFalseTarget(); + bDest = block->GetTrueTarget(); + bFalseDest = block->GetFalseTarget(); if (bFalseDest->KindIs(BBJ_ALWAYS) && bFalseDest->TargetIs(bDest) && bFalseDest->isEmpty()) { // Optimize block -> bFalseDest -> bDest into a BBJ_ALWAYS block->SetFalseTarget(bDest); fgAddRefPred(bDest, block, fgRemoveRefPred(bFalseDest, block)); + + // We will convert block to BBJ_ALWAYS, so no need to keep bFalseDest up to date } else if (bDest->KindIs(BBJ_ALWAYS) && bDest->TargetIs(bFalseDest) && bDest->isEmpty()) { // Optimize block -> bDest -> bFalseDest into a BBJ_ALWAYS block->SetTrueTarget(bFalseDest); fgAddRefPred(bFalseDest, block, fgRemoveRefPred(bDest, block)); + + // We will continue to use bDest, so ensure it is up to date bDest = bFalseDest; } @@ -4835,8 +4841,9 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */, bool isPh { fgRemoveConditionalJump(block); assert(block->KindIs(BBJ_ALWAYS)); - change = true; - modified = true; + change = true; + modified = true; + bFalseDest = nullptr; } } @@ -4864,13 +4871,13 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */, bool isPh // (b) block jump target is elsewhere but join free, and // bNext's jump target has a join. // - if (block->KindIs(BBJ_COND) && // block is a BBJ_COND block - block->FalseTargetIs(bNext) && // false target is the next block - (bNext->bbRefs == 1) && // no other block jumps to bNext - bNext->KindIs(BBJ_ALWAYS) && // the next block is a BBJ_ALWAYS block - !bNext->JumpsToNext() && // and it doesn't jump to the next block (we might compact them) - bNext->isEmpty() && // and it is an empty block - !bNext->TargetIs(bNext) && // special case for self jumps + if (block->KindIs(BBJ_COND) && // block is a BBJ_COND block + (bFalseDest == bNext) && // false target is the next block + (bNext->bbRefs == 1) && // no other block jumps to bNext + bNext->KindIs(BBJ_ALWAYS) && // the next block is a BBJ_ALWAYS block + !bNext->JumpsToNext() && // and it doesn't jump to the next block (we might compact them) + bNext->isEmpty() && // and it is an empty block + !bNext->TargetIs(bNext) && // special case for self jumps !bDest->IsFirstColdBlock(this) && !fgInDifferentRegions(block, bDest)) // do not cross hot/cold sections {