From 31aad96e0338e6679168a27bb43cc78ccd8c57f1 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 23 Mar 2021 10:52:53 +0300 Subject: [PATCH 01/11] Remove weights from fgExpandQmarkForCastInstOf --- src/coreclr/jit/morph.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index de5f6d0bdd2989..7745fbecfeb2db 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -17764,12 +17764,6 @@ void Compiler::fgExpandQmarkForCastInstOf(BasicBlock* block, Statement* stmt) cond1Block->bbJumpDest = remainderBlock; cond2Block->bbJumpDest = remainderBlock; - // Set the weights; some are guesses. - asgBlock->inheritWeight(block); - cond1Block->inheritWeight(block); - cond2Block->inheritWeightPercentage(cond1Block, 50); - helperBlock->inheritWeightPercentage(cond2Block, 50); - // Append cond1 as JTRUE to cond1Block GenTree* jmpTree = gtNewOperNode(GT_JTRUE, TYP_VOID, condExpr); Statement* jmpStmt = fgNewStmtFromTree(jmpTree, stmt->GetILOffsetX()); From 697ff0c3cd5eaa6df10b42c436409ff03900021a Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 26 Mar 2021 02:03:18 +0300 Subject: [PATCH 02/11] Guess weights from preds and successors --- src/coreclr/jit/morph.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 6909ab829eaf99..988e544ef3fa73 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -17681,6 +17681,9 @@ void Compiler::fgExpandQmarkForCastInstOf(BasicBlock* block, Statement* stmt) assert(qmark->gtFlags & GTF_QMARK_CAST_INSTOF); + float currBbWeight = block->bbWeight; + float nextBbWeight = (block->bbNext != nullptr) ? block->bbNext->bbWeight : currBbWeight; + // Get cond, true, false exprs for the qmark. GenTree* condExpr = qmark->gtGetOp1(); GenTree* trueExpr = qmark->gtGetOp2()->AsColon()->ThenNode(); @@ -17766,6 +17769,25 @@ void Compiler::fgExpandQmarkForCastInstOf(BasicBlock* block, Statement* stmt) cond1Block->bbJumpDest = remainderBlock; cond2Block->bbJumpDest = remainderBlock; + // Set the weights; some are guesses. + asgBlock->inheritWeight(block); + cond1Block->inheritWeight(block); + + // We shouldn't expand casts inside cold blocks in the first place. + assert(currBbWeight > 0); + + // Currently, we don't instrument internal blocks, so the only way we can set weights to these blocks + // is to analyze successors and guess. + + // cond2Block [0.5 .. 1] + const float cond2BlockWeight = 50.0f * nextBbWeight / currBbWeight + 50.0f; + + // helperBlock [0 .. 1] + const float helperBlockWeight = nextBbWeight / currBbWeight * 100.0f; + + cond2Block->inheritWeightPercentage(cond1Block, (UINT32)cond2BlockWeight); + helperBlock->inheritWeightPercentage(cond2Block, (UINT32)helperBlockWeight); + // Append cond1 as JTRUE to cond1Block GenTree* jmpTree = gtNewOperNode(GT_JTRUE, TYP_VOID, condExpr); Statement* jmpStmt = fgNewStmtFromTree(jmpTree, stmt->GetILOffsetX()); From d94791b0418134ce3b8dc68b9b94272f3c496fe0 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 26 Mar 2021 02:26:57 +0300 Subject: [PATCH 03/11] clean up --- src/coreclr/jit/morph.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 988e544ef3fa73..ede313d88426c5 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -17778,15 +17778,11 @@ void Compiler::fgExpandQmarkForCastInstOf(BasicBlock* block, Statement* stmt) // Currently, we don't instrument internal blocks, so the only way we can set weights to these blocks // is to analyze successors and guess. + const float cond2BlockWeight = max(50.0f * nextBbWeight / currBbWeight + 50.0f, 50.0f); + const float helperBlockWeight = max(nextBbWeight * 100.0f / currBbWeight, 100.0f); - // cond2Block [0.5 .. 1] - const float cond2BlockWeight = 50.0f * nextBbWeight / currBbWeight + 50.0f; - - // helperBlock [0 .. 1] - const float helperBlockWeight = nextBbWeight / currBbWeight * 100.0f; - - cond2Block->inheritWeightPercentage(cond1Block, (UINT32)cond2BlockWeight); - helperBlock->inheritWeightPercentage(cond2Block, (UINT32)helperBlockWeight); + cond2Block->inheritWeightPercentage(block, (UINT32)cond2BlockWeight); + helperBlock->inheritWeightPercentage(block, (UINT32)helperBlockWeight); // Append cond1 as JTRUE to cond1Block GenTree* jmpTree = gtNewOperNode(GT_JTRUE, TYP_VOID, condExpr); From b43a364566811d87f01f13a93e208394d4baad62 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 26 Mar 2021 11:26:15 +0300 Subject: [PATCH 04/11] Change isRunRarely to bbWeight --- src/coreclr/jit/importer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 005871f7740b74..202381a64e82d3 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -10957,7 +10957,7 @@ GenTree* Compiler::impCastClassOrIsInstToTree(GenTree* op1, // Don't bother with inline expansion when jit is trying to // generate code quickly, or the cast is in code that won't run very // often, or the method already is pretty big. - if (compCurBB->isRunRarely() || opts.OptimizationDisabled()) + if ((compCurBB->bbWeight <= BB_ZERO_WEIGHT) || opts.OptimizationDisabled()) { // not worth the code expansion if jitting fast or in a rarely run block shouldExpandInline = false; From 3774ed9ecf3f5153e40299e10247c1f00121615f Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 26 Mar 2021 12:39:35 +0300 Subject: [PATCH 05/11] add clamp --- src/coreclr/jit/morph.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index ede313d88426c5..25be59d30758bf 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -17778,8 +17778,8 @@ void Compiler::fgExpandQmarkForCastInstOf(BasicBlock* block, Statement* stmt) // Currently, we don't instrument internal blocks, so the only way we can set weights to these blocks // is to analyze successors and guess. - const float cond2BlockWeight = max(50.0f * nextBbWeight / currBbWeight + 50.0f, 50.0f); - const float helperBlockWeight = max(nextBbWeight * 100.0f / currBbWeight, 100.0f); + const float cond2BlockWeight = min(max(50.0f * nextBbWeight / currBbWeight + 50.0f, 50.0f), 100.0f); + const float helperBlockWeight = min(max(nextBbWeight * 100.0f / currBbWeight, 0.0f), 100.0f); cond2Block->inheritWeightPercentage(block, (UINT32)cond2BlockWeight); helperBlock->inheritWeightPercentage(block, (UINT32)helperBlockWeight); From 625a42a60194ec7147ccbc422264ba4770d1f5a2 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 26 Mar 2021 14:32:36 +0300 Subject: [PATCH 06/11] fix ci --- src/coreclr/jit/importer.cpp | 2 +- src/coreclr/jit/morph.cpp | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 202381a64e82d3..005871f7740b74 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -10957,7 +10957,7 @@ GenTree* Compiler::impCastClassOrIsInstToTree(GenTree* op1, // Don't bother with inline expansion when jit is trying to // generate code quickly, or the cast is in code that won't run very // often, or the method already is pretty big. - if ((compCurBB->bbWeight <= BB_ZERO_WEIGHT) || opts.OptimizationDisabled()) + if (compCurBB->isRunRarely() || opts.OptimizationDisabled()) { // not worth the code expansion if jitting fast or in a rarely run block shouldExpandInline = false; diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 25be59d30758bf..30a4260dcb535e 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -17773,13 +17773,15 @@ void Compiler::fgExpandQmarkForCastInstOf(BasicBlock* block, Statement* stmt) asgBlock->inheritWeight(block); cond1Block->inheritWeight(block); - // We shouldn't expand casts inside cold blocks in the first place. - assert(currBbWeight > 0); - // Currently, we don't instrument internal blocks, so the only way we can set weights to these blocks // is to analyze successors and guess. - const float cond2BlockWeight = min(max(50.0f * nextBbWeight / currBbWeight + 50.0f, 50.0f), 100.0f); - const float helperBlockWeight = min(max(nextBbWeight * 100.0f / currBbWeight, 0.0f), 100.0f); + float cond2BlockWeight = 0; + float helperBlockWeight = 0; + if (currBbWeight > 0) + { + cond2BlockWeight = min(max(50.0f * nextBbWeight / currBbWeight + 50.0f, 50.0f), 100.0f); + helperBlockWeight = min(max(nextBbWeight * 100.0f / currBbWeight, 0.0f), 100.0f); + } cond2Block->inheritWeightPercentage(block, (UINT32)cond2BlockWeight); helperBlock->inheritWeightPercentage(block, (UINT32)helperBlockWeight); From ff71dafd1f42762f8f8d8438dea63b5f21b3f4c9 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 26 Mar 2021 20:40:33 +0300 Subject: [PATCH 07/11] Better weights --- src/coreclr/jit/morph.cpp | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 30a4260dcb535e..1ed122184f9bd6 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -17769,22 +17769,18 @@ void Compiler::fgExpandQmarkForCastInstOf(BasicBlock* block, Statement* stmt) cond1Block->bbJumpDest = remainderBlock; cond2Block->bbJumpDest = remainderBlock; - // Set the weights; some are guesses. - asgBlock->inheritWeight(block); - cond1Block->inheritWeight(block); - // Currently, we don't instrument internal blocks, so the only way we can set weights to these blocks - // is to analyze successors and guess. - float cond2BlockWeight = 0; - float helperBlockWeight = 0; + // is to analyze successors and take a guess. + float cond2BlockWeight = 0; if (currBbWeight > 0) { - cond2BlockWeight = min(max(50.0f * nextBbWeight / currBbWeight + 50.0f, 50.0f), 100.0f); - helperBlockWeight = min(max(nextBbWeight * 100.0f / currBbWeight, 0.0f), 100.0f); + cond2BlockWeight = min(max(50.0f * nextBbWeight / currBbWeight + 50.0f, 50.0f), 100.0f); } + asgBlock->inheritWeight(block); + cond1Block->inheritWeight(block); cond2Block->inheritWeightPercentage(block, (UINT32)cond2BlockWeight); - helperBlock->inheritWeightPercentage(block, (UINT32)helperBlockWeight); + helperBlock->inheritWeight(cond2Block); // Append cond1 as JTRUE to cond1Block GenTree* jmpTree = gtNewOperNode(GT_JTRUE, TYP_VOID, condExpr); From 9efadff4935fc688d0759e6058cb5bb909a59a64 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Mon, 29 Mar 2021 20:18:35 +0300 Subject: [PATCH 08/11] Update morph.cpp --- src/coreclr/jit/morph.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 1ed122184f9bd6..d64724c91a7cc2 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -17780,8 +17780,8 @@ void Compiler::fgExpandQmarkForCastInstOf(BasicBlock* block, Statement* stmt) asgBlock->inheritWeight(block); cond1Block->inheritWeight(block); cond2Block->inheritWeightPercentage(block, (UINT32)cond2BlockWeight); - helperBlock->inheritWeight(cond2Block); - + helperBlock->inheritWeightPercentage(block, 100 - (UINT32)cond2BlockWeight); + // Append cond1 as JTRUE to cond1Block GenTree* jmpTree = gtNewOperNode(GT_JTRUE, TYP_VOID, condExpr); Statement* jmpStmt = fgNewStmtFromTree(jmpTree, stmt->GetILOffsetX()); From 6bfd18844d57981357ac28a292a8b5d436d4354c Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Mon, 29 Mar 2021 20:45:21 +0300 Subject: [PATCH 09/11] Update morph.cpp --- src/coreclr/jit/morph.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index d64724c91a7cc2..1bf9215cdaea9c 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -17781,7 +17781,7 @@ void Compiler::fgExpandQmarkForCastInstOf(BasicBlock* block, Statement* stmt) cond1Block->inheritWeight(block); cond2Block->inheritWeightPercentage(block, (UINT32)cond2BlockWeight); helperBlock->inheritWeightPercentage(block, 100 - (UINT32)cond2BlockWeight); - + // Append cond1 as JTRUE to cond1Block GenTree* jmpTree = gtNewOperNode(GT_JTRUE, TYP_VOID, condExpr); Statement* jmpStmt = fgNewStmtFromTree(jmpTree, stmt->GetILOffsetX()); From c24ac4d48fb3a960eeb165fae0ef305f34e39569 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 1 Apr 2021 15:10:08 +0300 Subject: [PATCH 10/11] Address feedback --- src/coreclr/jit/morph.cpp | 25 ++++++++++++++++++------- src/coreclr/jit/utils.h | 7 +++++++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 56bc05ce59d0cf..cce513a4c01d8d 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -17697,8 +17697,8 @@ void Compiler::fgExpandQmarkForCastInstOf(BasicBlock* block, Statement* stmt) assert(qmark->gtFlags & GTF_QMARK_CAST_INSTOF); - float currBbWeight = block->bbWeight; - float nextBbWeight = (block->bbNext != nullptr) ? block->bbNext->bbWeight : currBbWeight; + const BasicBlock::weight_t currBbWeight = block->bbWeight; + const BasicBlock::weight_t nextBbWeight = (block->bbNext != nullptr) ? block->bbNext->bbWeight : currBbWeight; // Get cond, true, false exprs for the qmark. GenTree* condExpr = qmark->gtGetOp1(); @@ -17787,16 +17787,27 @@ void Compiler::fgExpandQmarkForCastInstOf(BasicBlock* block, Statement* stmt) // Currently, we don't instrument internal blocks, so the only way we can set weights to these blocks // is to analyze successors and take a guess. - float cond2BlockWeight = 0; - if (currBbWeight > 0) + UINT32 cond2BlockLikelihood = BB_ZERO_WEIGHT; + UINT32 helperBlockLikelihood = BB_ZERO_WEIGHT; + + // We don't expand casts inside rarely executed blocks, but currently we skip only blocks + // with BBF_RUN_RARELY flag and still can hit zero weight here (in that cases all internal blocks + // will also have zero weight). + if (currBbWeight > BB_ZERO_WEIGHT) { - cond2BlockWeight = min(max(50.0f * nextBbWeight / currBbWeight + 50.0f, 50.0f), 100.0f); + const BasicBlock::weight_t castSuccessLikelihood = clamp(nextBbWeight / currBbWeight, 0.0f, 1.0f); + + // cond2Block is always taken if the cast always succeeds (helperBlock will be cold in this case) + // If it always fails the only guess we can make that it's either object is null or of a + // wrong type (50/50). + cond2BlockLikelihood = (UINT32)(castSuccessLikelihood * 50.0f) + 50; + helperBlockLikelihood = 100 - cond2BlockLikelihood; } asgBlock->inheritWeight(block); cond1Block->inheritWeight(block); - cond2Block->inheritWeightPercentage(block, (UINT32)cond2BlockWeight); - helperBlock->inheritWeightPercentage(block, 100 - (UINT32)cond2BlockWeight); + cond2Block->inheritWeightPercentage(block, cond2BlockLikelihood); + helperBlock->inheritWeightPercentage(block, helperBlockLikelihood); // Append cond1 as JTRUE to cond1Block GenTree* jmpTree = gtNewOperNode(GT_JTRUE, TYP_VOID, condExpr); diff --git a/src/coreclr/jit/utils.h b/src/coreclr/jit/utils.h index e22320bb1ef635..0a0fb1dd05d2f3 100644 --- a/src/coreclr/jit/utils.h +++ b/src/coreclr/jit/utils.h @@ -41,6 +41,13 @@ inline bool isPow2(T i) return (i > 0 && ((i - 1) & i) == 0); } +// Clamps the given value between the given lower and upper values +template +inline bool clamp(T value, T lower, T upper) +{ + return max(lower, min(value, upper)); +} + // Adapter for iterators to a type that is compatible with C++11 // range-based for loops. template From a2f9ba91bd30bc1d2617478d61f572acdbb72785 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Thu, 1 Apr 2021 15:43:40 +0300 Subject: [PATCH 11/11] Update morph.cpp --- src/coreclr/jit/morph.cpp | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index cce513a4c01d8d..8d7709fc7e6e6b 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -17787,27 +17787,24 @@ void Compiler::fgExpandQmarkForCastInstOf(BasicBlock* block, Statement* stmt) // Currently, we don't instrument internal blocks, so the only way we can set weights to these blocks // is to analyze successors and take a guess. - UINT32 cond2BlockLikelihood = BB_ZERO_WEIGHT; - UINT32 helperBlockLikelihood = BB_ZERO_WEIGHT; + BasicBlock::weight_t castSuccessLikelihood = BB_ZERO_WEIGHT; // We don't expand casts inside rarely executed blocks, but currently we skip only blocks // with BBF_RUN_RARELY flag and still can hit zero weight here (in that cases all internal blocks // will also have zero weight). if (currBbWeight > BB_ZERO_WEIGHT) { - const BasicBlock::weight_t castSuccessLikelihood = clamp(nextBbWeight / currBbWeight, 0.0f, 1.0f); - - // cond2Block is always taken if the cast always succeeds (helperBlock will be cold in this case) - // If it always fails the only guess we can make that it's either object is null or of a - // wrong type (50/50). - cond2BlockLikelihood = (UINT32)(castSuccessLikelihood * 50.0f) + 50; - helperBlockLikelihood = 100 - cond2BlockLikelihood; + castSuccessLikelihood = clamp(nextBbWeight / currBbWeight, BB_ZERO_WEIGHT, 1.0f); } asgBlock->inheritWeight(block); cond1Block->inheritWeight(block); - cond2Block->inheritWeightPercentage(block, cond2BlockLikelihood); - helperBlock->inheritWeightPercentage(block, helperBlockLikelihood); + + // cond2Block is always taken if the cast always succeeds (helperBlock will be cold in this case) + // If it always fails the only guess we can make that it's either object is null or of a + // wrong type (50/50). + cond2Block->inheritWeightPercentage(block, (UINT32)(castSuccessLikelihood * 50.0f) + 50); + helperBlock->inheritWeightPercentage(block, 50 - (UINT32)(castSuccessLikelihood * 50.0f)); // Append cond1 as JTRUE to cond1Block GenTree* jmpTree = gtNewOperNode(GT_JTRUE, TYP_VOID, condExpr);