From 593a3ece7d8177f094171b64686b7df8a13e49e4 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 7 Jun 2022 20:04:19 +0200 Subject: [PATCH 01/10] Do not propagate some constants --- src/coreclr/jit/assertionprop.cpp | 35 +++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index ddee8ba3e6e1b4..3cfbfde2ea65df 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3177,6 +3177,41 @@ GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* tree) if (conValTree != nullptr) { + if (tree->OperIs(GT_LCL_VAR)) + { + bool betterToPropagate = false; + if (conValTree->IsVectorZero() || conValTree->IsFloatPositiveZero()) + { + // These are cheap to re-materialize (unlike e.g. "1.0" or "{1,1,1,1}" which end up as + // memory loads from the data section) + betterToPropagate = true; + } +#ifdef TARGET_ARM64 + else if (conValTree->IsCnsIntOrI() && unsigned_abs(conValTree->AsIntCon()->IconValue()) < 0x0fff) + { + // This integer constant is likely immable + betterToPropagate = true; + } +#endif + + if (!betterToPropagate) + { + // Try to find the block this value was defined in + // and if the current use is inside a loop while the def is not - do not propagate + unsigned lclNum = tree->AsLclVarCommon()->GetLclNum(); + unsigned ssaNum = GetSsaNumForLocalVarDef(tree); + if (ssaNum != SsaConfig::RESERVED_SSA_NUM) + { + BasicBlock* defBlock = lvaTable[lclNum].GetPerSsaData(ssaNum)->GetBlock(); + if (defBlock != nullptr && !(defBlock->bbFlags & BBF_BACKWARD_JUMP) && + (block->bbFlags & BBF_BACKWARD_JUMP)) + { + return nullptr; + } + } + } + } + // Were able to optimize. conValTree->gtVNPair = vnPair; GenTree* sideEffList = optExtractSideEffListFromConst(tree); From 72ab5793039baf05c615ec2ac505acad5646f389 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 8 Jun 2022 16:09:43 +0200 Subject: [PATCH 02/10] Limit to vectors for now --- src/coreclr/jit/assertionprop.cpp | 42 +++++++++++++++---------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 3cfbfde2ea65df..ebe41bd4c4d95f 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3179,34 +3179,32 @@ GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* tree) { if (tree->OperIs(GT_LCL_VAR)) { - bool betterToPropagate = false; - if (conValTree->IsVectorZero() || conValTree->IsFloatPositiveZero()) - { - // These are cheap to re-materialize (unlike e.g. "1.0" or "{1,1,1,1}" which end up as - // memory loads from the data section) - betterToPropagate = true; - } -#ifdef TARGET_ARM64 - else if (conValTree->IsCnsIntOrI() && unsigned_abs(conValTree->AsIntCon()->IconValue()) < 0x0fff) - { - // This integer constant is likely immable - betterToPropagate = true; - } -#endif - - if (!betterToPropagate) + if (conValTree->OperIs(GT_CNS_VEC) && !conValTree->IsVectorZero() && !conValTree->IsVectorAllBitsSet()) { // Try to find the block this value was defined in - // and if the current use is inside a loop while the def is not - do not propagate - unsigned lclNum = tree->AsLclVarCommon()->GetLclNum(); - unsigned ssaNum = GetSsaNumForLocalVarDef(tree); + const unsigned lclNum = tree->AsLclVarCommon()->GetLclNum(); + const unsigned ssaNum = GetSsaNumForLocalVarDef(tree); if (ssaNum != SsaConfig::RESERVED_SSA_NUM) { BasicBlock* defBlock = lvaTable[lclNum].GetPerSsaData(ssaNum)->GetBlock(); - if (defBlock != nullptr && !(defBlock->bbFlags & BBF_BACKWARD_JUMP) && - (block->bbFlags & BBF_BACKWARD_JUMP)) + + // If the vector constant was defined outside of the loop - do not propagate + // TODO: do propagate if it lives across a call in that loop and the ABI doesn't have + // callee-saved registers + if (defBlock != nullptr) { - return nullptr; + const weight_t defBlockWeight = defBlock->getBBWeight(this); + const weight_t blockWeight = block->getBBWeight(this); + + // A simple heuristic: If the constant is defined outside the loop (not far from its head) + // and is used inside it - don't propagate. + // TODO: if it lives accross calls in that loops we'd better propagate it after those + // calls on ABIs without callee-saved registers (e.g. Linux-x64) + if ((defBlockWeight > 0) && (blockWeight / defBlockWeight) >= 2) + { + JITDUMP("Skip constant propagation of vector constant inside loop " FMT_BB "\n", block->bbNum); + return nullptr; + } } } } From 83fc086fdad80692608e0f71f5e772a182bf8e99 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 8 Jun 2022 22:01:55 +0200 Subject: [PATCH 03/10] Extend impl for other constants --- src/coreclr/jit/assertionprop.cpp | 75 ++++++++++++++++++++++++------- src/coreclr/jit/block.h | 5 +++ 2 files changed, 65 insertions(+), 15 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index ebe41bd4c4d95f..5669041b6da7ed 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3177,33 +3177,78 @@ GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* tree) if (conValTree != nullptr) { - if (tree->OperIs(GT_LCL_VAR)) + if (tree->OperIs(GT_LCL_VAR) && conValTree->OperIs(GT_CNS_VEC, GT_CNS_DBL, GT_CNS_INT)) { - if (conValTree->OperIs(GT_CNS_VEC) && !conValTree->IsVectorZero() && !conValTree->IsVectorAllBitsSet()) + bool keepPropagating = false; + + if (conValTree->OperIs(GT_CNS_VEC)) + { + if (conValTree->IsVectorZero() || conValTree->IsVectorAllBitsSet()) + { + keepPropagating = true; + } + } + else if (conValTree->OperIs(GT_CNS_DBL)) + { + if (conValTree->IsFloatPositiveZero()) + { + keepPropagating = true; + } +#ifdef TARGET_ARM64 + if (emitter::canEncodeFloatImm8(conValTree->AsDblCon()->gtDconVal)) + { + // This float won't be expensive for the loop + keepPropagating = true; + } +#endif + } + else { - // Try to find the block this value was defined in + assert(conValTree->OperIs(GT_CNS_INT)); +#ifdef TARGET_ARM64 + if ((unsigned_abs(conValTree->AsIntCon()->IconValue()) <= 0xFFF) || + isPow2(conValTree->AsIntCon()->IconValue())) + { + keepPropagating = true; + } +#endif + } + + if (!keepPropagating) + { + // Try to find the block this constant was originally defined in const unsigned lclNum = tree->AsLclVarCommon()->GetLclNum(); const unsigned ssaNum = GetSsaNumForLocalVarDef(tree); if (ssaNum != SsaConfig::RESERVED_SSA_NUM) { BasicBlock* defBlock = lvaTable[lclNum].GetPerSsaData(ssaNum)->GetBlock(); - // If the vector constant was defined outside of the loop - do not propagate - // TODO: do propagate if it lives across a call in that loop and the ABI doesn't have - // callee-saved registers if (defBlock != nullptr) { - const weight_t defBlockWeight = defBlock->getBBWeight(this); - const weight_t blockWeight = block->getBBWeight(this); - - // A simple heuristic: If the constant is defined outside the loop (not far from its head) + // A simple heuristic: If the constant is defined outside of a loop (not far from its head) // and is used inside it - don't propagate. - // TODO: if it lives accross calls in that loops we'd better propagate it after those - // calls on ABIs without callee-saved registers (e.g. Linux-x64) - if ((defBlockWeight > 0) && (blockWeight / defBlockWeight) >= 2) + + // TODO: if it lives across calls in that loops we'd better propagate it after those + // calls on ABIs without callee-saved registers + + if (!defBlock->IsInLoop() && block->IsInLoop()) { - JITDUMP("Skip constant propagation of vector constant inside loop " FMT_BB "\n", block->bbNum); - return nullptr; + //const LoopDsc* loopInfo = &optLoopTable[block->bbNatLoopNum]; + // Definition better to be close (e.g. result of CSE) so we won't occupy a register + + //if ((loopInfo->lpHead == defBlock) || loopInfo->lpHead->bbIDom == defBlock) + { + // Last, let's limit it to cases where block is not some not-always-taken + // block inside that loop + //weight_t defBlockWeight = defBlock->getBBWeight(this); + //weight_t blockWeight = block->getBBWeight(this); + + //if (((blockWeight / defBlockWeight) >= BB_LOOP_WEIGHT_SCALE) && (defBlockWeight > 0)) + { + JITDUMP("Skip constant propagation inside loop " FMT_BB "\n", block->bbNum); + return nullptr; + } + } } } } diff --git a/src/coreclr/jit/block.h b/src/coreclr/jit/block.h index 88dcb79794fb87..7a2fd187cf321b 100644 --- a/src/coreclr/jit/block.h +++ b/src/coreclr/jit/block.h @@ -1077,6 +1077,11 @@ struct BasicBlock : private LIR::Range return PredEdgeList(bbPreds); } + bool IsInLoop() const + { + return bbNatLoopNum != NOT_IN_LOOP; + } + // PredBlocks: convenience method for enabling range-based `for` iteration over predecessor blocks, e.g.: // for (BasicBlock* const predBlock : block->PredBlocks()) ... // From d81b1958434f9a449866cb74e0ae0c9eb19b4e56 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 8 Jun 2022 23:29:16 +0200 Subject: [PATCH 04/10] Clean up --- src/coreclr/jit/assertionprop.cpp | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 5669041b6da7ed..f567fe89d5b18a 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3179,12 +3179,16 @@ GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* tree) { if (tree->OperIs(GT_LCL_VAR) && conValTree->OperIs(GT_CNS_VEC, GT_CNS_DBL, GT_CNS_INT)) { + // A simple heuristic: If the constant is defined outside of a loop (not far from its head) + // and is used inside it - don't propagate. + bool keepPropagating = false; if (conValTree->OperIs(GT_CNS_VEC)) { if (conValTree->IsVectorZero() || conValTree->IsVectorAllBitsSet()) { + // These are cheap to materialize keepPropagating = true; } } @@ -3192,12 +3196,13 @@ GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* tree) { if (conValTree->IsFloatPositiveZero()) { + // This is cheap to materialize keepPropagating = true; } #ifdef TARGET_ARM64 if (emitter::canEncodeFloatImm8(conValTree->AsDblCon()->gtDconVal)) { - // This float won't be expensive for the loop + // Such floats are likely immable on arm64 keepPropagating = true; } #endif @@ -3209,8 +3214,12 @@ GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* tree) if ((unsigned_abs(conValTree->AsIntCon()->IconValue()) <= 0xFFF) || isPow2(conValTree->AsIntCon()->IconValue())) { + // Small integers are likely containable on arm keepPropagating = true; } +#elif TARGET_XARCH + // All integer constants are cheap to materialize on xarch + keepPropagating = true; #endif } @@ -3225,25 +3234,22 @@ GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* tree) if (defBlock != nullptr) { - // A simple heuristic: If the constant is defined outside of a loop (not far from its head) - // and is used inside it - don't propagate. - // TODO: if it lives across calls in that loops we'd better propagate it after those - // calls on ABIs without callee-saved registers + // TODO: if it lives across calls in that loops we should propagate it only after those + // calls on ABIs without callee-saved registers to avoid spills if (!defBlock->IsInLoop() && block->IsInLoop()) { - //const LoopDsc* loopInfo = &optLoopTable[block->bbNatLoopNum]; - // Definition better to be close (e.g. result of CSE) so we won't occupy a register - - //if ((loopInfo->lpHead == defBlock) || loopInfo->lpHead->bbIDom == defBlock) + const LoopDsc* loopInfo = &optLoopTable[block->bbNatLoopNum]; + // Def is better to be close (e.g. result of CSE) so we won't occupy a register + if ((loopInfo->lpHead == defBlock) || loopInfo->lpHead->bbIDom == defBlock) { // Last, let's limit it to cases where block is not some not-always-taken // block inside that loop - //weight_t defBlockWeight = defBlock->getBBWeight(this); - //weight_t blockWeight = block->getBBWeight(this); + const weight_t defBlockWeight = defBlock->getBBWeight(this); + const weight_t blockWeight = block->getBBWeight(this); - //if (((blockWeight / defBlockWeight) >= BB_LOOP_WEIGHT_SCALE) && (defBlockWeight > 0)) + if (((blockWeight / defBlockWeight) >= BB_LOOP_WEIGHT_SCALE) && (defBlockWeight > 0)) { JITDUMP("Skip constant propagation inside loop " FMT_BB "\n", block->bbNum); return nullptr; From 1d076dde87c05ac5cc9bce5afc17020dbe2d3d79 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 9 Jun 2022 00:38:37 +0200 Subject: [PATCH 05/10] Remove integers for now --- src/coreclr/jit/assertionprop.cpp | 43 +++++++------------------------ 1 file changed, 10 insertions(+), 33 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index f567fe89d5b18a..5c2924ed52d3a5 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3177,7 +3177,7 @@ GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* tree) if (conValTree != nullptr) { - if (tree->OperIs(GT_LCL_VAR) && conValTree->OperIs(GT_CNS_VEC, GT_CNS_DBL, GT_CNS_INT)) + if (tree->OperIs(GT_LCL_VAR) && conValTree->OperIs(GT_CNS_VEC, GT_CNS_DBL)) { // A simple heuristic: If the constant is defined outside of a loop (not far from its head) // and is used inside it - don't propagate. @@ -3199,7 +3199,7 @@ GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* tree) // This is cheap to materialize keepPropagating = true; } -#ifdef TARGET_ARM64 +#if defined(TARGET_ARM64) if (emitter::canEncodeFloatImm8(conValTree->AsDblCon()->gtDconVal)) { // Such floats are likely immable on arm64 @@ -3207,21 +3207,6 @@ GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* tree) } #endif } - else - { - assert(conValTree->OperIs(GT_CNS_INT)); -#ifdef TARGET_ARM64 - if ((unsigned_abs(conValTree->AsIntCon()->IconValue()) <= 0xFFF) || - isPow2(conValTree->AsIntCon()->IconValue())) - { - // Small integers are likely containable on arm - keepPropagating = true; - } -#elif TARGET_XARCH - // All integer constants are cheap to materialize on xarch - keepPropagating = true; -#endif - } if (!keepPropagating) { @@ -3231,29 +3216,21 @@ GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* tree) if (ssaNum != SsaConfig::RESERVED_SSA_NUM) { BasicBlock* defBlock = lvaTable[lclNum].GetPerSsaData(ssaNum)->GetBlock(); - if (defBlock != nullptr) { - // TODO: if it lives across calls in that loops we should propagate it only after those // calls on ABIs without callee-saved registers to avoid spills - if (!defBlock->IsInLoop() && block->IsInLoop()) { - const LoopDsc* loopInfo = &optLoopTable[block->bbNatLoopNum]; - // Def is better to be close (e.g. result of CSE) so we won't occupy a register - if ((loopInfo->lpHead == defBlock) || loopInfo->lpHead->bbIDom == defBlock) + // Last, let's limit it to cases where block is not some not-always-taken + // block inside that loop + const weight_t defBlockWeight = defBlock->getBBWeight(this); + const weight_t blockWeight = block->getBBWeight(this); + + if ((defBlockWeight > 0) && ((blockWeight / defBlockWeight) >= BB_LOOP_WEIGHT_SCALE)) { - // Last, let's limit it to cases where block is not some not-always-taken - // block inside that loop - const weight_t defBlockWeight = defBlock->getBBWeight(this); - const weight_t blockWeight = block->getBBWeight(this); - - if (((blockWeight / defBlockWeight) >= BB_LOOP_WEIGHT_SCALE) && (defBlockWeight > 0)) - { - JITDUMP("Skip constant propagation inside loop " FMT_BB "\n", block->bbNum); - return nullptr; - } + JITDUMP("Skip constant propagation inside loop " FMT_BB "\n", block->bbNum); + return nullptr; } } } From 69e4d1ddfa3581629750cd24c2f4f37e2119d20d Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 9 Jun 2022 21:24:55 +0200 Subject: [PATCH 06/10] Address feedback --- src/coreclr/jit/assertionprop.cpp | 57 +++++++++---------------------- 1 file changed, 16 insertions(+), 41 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 5c2924ed52d3a5..013614badf5807 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3177,61 +3177,36 @@ GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* tree) if (conValTree != nullptr) { + // TODO: Extend on integers, especially on ARM where a constant might take up to 4 instructions if (tree->OperIs(GT_LCL_VAR) && conValTree->OperIs(GT_CNS_VEC, GT_CNS_DBL)) { // A simple heuristic: If the constant is defined outside of a loop (not far from its head) // and is used inside it - don't propagate. - bool keepPropagating = false; - - if (conValTree->OperIs(GT_CNS_VEC)) + if (!conValTree->gtCostsInitialized) { - if (conValTree->IsVectorZero() || conValTree->IsVectorAllBitsSet()) - { - // These are cheap to materialize - keepPropagating = true; - } - } - else if (conValTree->OperIs(GT_CNS_DBL)) - { - if (conValTree->IsFloatPositiveZero()) - { - // This is cheap to materialize - keepPropagating = true; - } -#if defined(TARGET_ARM64) - if (emitter::canEncodeFloatImm8(conValTree->AsDblCon()->gtDconVal)) - { - // Such floats are likely immable on arm64 - keepPropagating = true; - } -#endif + gtPrepareCost(conValTree); } - if (!keepPropagating) + if ((conValTree->GetCostEx() > 1) && (conValTree->GetCostSz() > 1)) { // Try to find the block this constant was originally defined in - const unsigned lclNum = tree->AsLclVarCommon()->GetLclNum(); - const unsigned ssaNum = GetSsaNumForLocalVarDef(tree); - if (ssaNum != SsaConfig::RESERVED_SSA_NUM) + GenTreeLclVar* lcl = tree->AsLclVar(); + if (lcl->HasSsaName()) { - BasicBlock* defBlock = lvaTable[lclNum].GetPerSsaData(ssaNum)->GetBlock(); + BasicBlock* defBlock = lvaGetDesc(lcl)->GetPerSsaData(lcl->GetSsaNum())->GetBlock(); if (defBlock != nullptr) { - // TODO: if it lives across calls in that loops we should propagate it only after those - // calls on ABIs without callee-saved registers to avoid spills - if (!defBlock->IsInLoop() && block->IsInLoop()) + // Avoid propagating if the weighted use cost is significantly greater than the def cost. + // NOTE: this currently does not take "a float living across a call" case into account + // where we might end up with spill/restore on ABIs without callee-saved registers + const weight_t defBlockWeight = defBlock->getBBWeight(this); + const weight_t blockWeight = block->getBBWeight(this); + + if ((defBlockWeight > 0) && ((blockWeight / defBlockWeight) >= BB_LOOP_WEIGHT_SCALE)) { - // Last, let's limit it to cases where block is not some not-always-taken - // block inside that loop - const weight_t defBlockWeight = defBlock->getBBWeight(this); - const weight_t blockWeight = block->getBBWeight(this); - - if ((defBlockWeight > 0) && ((blockWeight / defBlockWeight) >= BB_LOOP_WEIGHT_SCALE)) - { - JITDUMP("Skip constant propagation inside loop " FMT_BB "\n", block->bbNum); - return nullptr; - } + JITDUMP("Skip constant propagation inside loop " FMT_BB "\n", block->bbNum); + return nullptr; } } } From 3920a5406e94c60ba7220525e2cd7a2a65e3dde2 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 9 Jun 2022 21:53:40 +0200 Subject: [PATCH 07/10] Move to a function --- src/coreclr/jit/assertionprop.cpp | 87 ++++++++++++++++++++----------- src/coreclr/jit/compiler.h | 1 + 2 files changed, 57 insertions(+), 31 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 013614badf5807..7d74579049e670 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3177,39 +3177,12 @@ GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* tree) if (conValTree != nullptr) { - // TODO: Extend on integers, especially on ARM where a constant might take up to 4 instructions - if (tree->OperIs(GT_LCL_VAR) && conValTree->OperIs(GT_CNS_VEC, GT_CNS_DBL)) + if (tree->OperIs(GT_LCL_VAR)) { - // A simple heuristic: If the constant is defined outside of a loop (not far from its head) - // and is used inside it - don't propagate. - - if (!conValTree->gtCostsInitialized) + if (!optIsProfitableToSubstitute(tree->AsLclVar(), block, conValTree)) { - gtPrepareCost(conValTree); - } - - if ((conValTree->GetCostEx() > 1) && (conValTree->GetCostSz() > 1)) - { - // Try to find the block this constant was originally defined in - GenTreeLclVar* lcl = tree->AsLclVar(); - if (lcl->HasSsaName()) - { - BasicBlock* defBlock = lvaGetDesc(lcl)->GetPerSsaData(lcl->GetSsaNum())->GetBlock(); - if (defBlock != nullptr) - { - // Avoid propagating if the weighted use cost is significantly greater than the def cost. - // NOTE: this currently does not take "a float living across a call" case into account - // where we might end up with spill/restore on ABIs without callee-saved registers - const weight_t defBlockWeight = defBlock->getBBWeight(this); - const weight_t blockWeight = block->getBBWeight(this); - - if ((defBlockWeight > 0) && ((blockWeight / defBlockWeight) >= BB_LOOP_WEIGHT_SCALE)) - { - JITDUMP("Skip constant propagation inside loop " FMT_BB "\n", block->bbNum); - return nullptr; - } - } - } + // Not profitable to substitute + return nullptr; } } @@ -3235,6 +3208,58 @@ GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* tree) } } +//------------------------------------------------------------------------------ +// optIsProfitableToPropagate: Checks if value worth substituting to lcl location +// +// Arguments: +// lcl - lcl to replace with value if profitable +// lclBlock - Basic block lcl located in +// value - value we plan to substitute to lcl +// +// Returns: +// False if it's likely not profitable to do substitution, True otherwise +// +bool Compiler::optIsProfitableToSubstitute(GenTreeLclVarCommon* lcl, BasicBlock* lclBlock, GenTree* value) +{ + // A simple heuristic: If the constant is defined outside of a loop (not far from its head) + // and is used inside it - don't propagate. + + // TODO: Extend on more kinds of trees + if (!value->OperIs(GT_CNS_VEC, GT_CNS_DBL)) + { + return true; + } + + if (!value->gtCostsInitialized) + { + gtPrepareCost(value); + } + + if ((value->GetCostEx() > 1) && (value->GetCostSz() > 1)) + { + // Try to find the block this constant was originally defined in + if (lcl->HasSsaName()) + { + BasicBlock* defBlock = lvaGetDesc(lcl)->GetPerSsaData(lcl->GetSsaNum())->GetBlock(); + if (defBlock != nullptr) + { + // Avoid propagating if the weighted use cost is significantly greater than the def cost. + // NOTE: this currently does not take "a float living across a call" case into account + // where we might end up with spill/restore on ABIs without callee-saved registers + const weight_t defBlockWeight = defBlock->getBBWeight(this); + const weight_t lclblockWeight = lclBlock->getBBWeight(this); + + if ((defBlockWeight > 0) && ((lclblockWeight / defBlockWeight) >= BB_LOOP_WEIGHT_SCALE)) + { + JITDUMP("Constant propagation inside loop " FMT_BB " is not profitable\n", lclBlock->bbNum); + return false; + } + } + } + } + return true; +} + //------------------------------------------------------------------------------ // optConstantAssertionProp: Possibly substitute a constant for a local use // diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index b4c7eb375b4d22..783b8be658a5f6 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7264,6 +7264,7 @@ class Compiler GenTree* optConstantAssertionProp(AssertionDsc* curAssertion, GenTreeLclVarCommon* tree, Statement* stmt DEBUGARG(AssertionIndex index)); + bool optIsProfitableToSubstitute(GenTreeLclVarCommon* lcl, BasicBlock* lclBlock, GenTree* value); bool optZeroObjAssertionProp(GenTree* tree, ASSERT_VALARG_TP assertions); // Assertion propagation functions. From 7873570d84697dfff1bea7e28fe61ae0cb98ecff Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 9 Jun 2022 21:54:30 +0200 Subject: [PATCH 08/10] Remove unused method --- src/coreclr/jit/block.h | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/coreclr/jit/block.h b/src/coreclr/jit/block.h index 7a2fd187cf321b..88dcb79794fb87 100644 --- a/src/coreclr/jit/block.h +++ b/src/coreclr/jit/block.h @@ -1077,11 +1077,6 @@ struct BasicBlock : private LIR::Range return PredEdgeList(bbPreds); } - bool IsInLoop() const - { - return bbNatLoopNum != NOT_IN_LOOP; - } - // PredBlocks: convenience method for enabling range-based `for` iteration over predecessor blocks, e.g.: // for (BasicBlock* const predBlock : block->PredBlocks()) ... // From bc40d087b0692b14c5940562d6d3baa524d9ec7e Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 9 Jun 2022 22:18:32 +0200 Subject: [PATCH 09/10] fix build --- src/coreclr/jit/assertionprop.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 7d74579049e670..15ec87549effd8 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3230,10 +3230,7 @@ bool Compiler::optIsProfitableToSubstitute(GenTreeLclVarCommon* lcl, BasicBlock* return true; } - if (!value->gtCostsInitialized) - { - gtPrepareCost(value); - } + gtPrepareCost(value); if ((value->GetCostEx() > 1) && (value->GetCostSz() > 1)) { From eaede4e0d148be0d865ebeb028e966b9db278628 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Fri, 10 Jun 2022 10:00:04 +0200 Subject: [PATCH 10/10] Update src/coreclr/jit/assertionprop.cpp Co-authored-by: Tanner Gooding --- src/coreclr/jit/assertionprop.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 15ec87549effd8..5ea3ba568406a3 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3209,7 +3209,7 @@ GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* tree) } //------------------------------------------------------------------------------ -// optIsProfitableToPropagate: Checks if value worth substituting to lcl location +// optIsProfitableToSubstitute: Checks if value worth substituting to lcl location // // Arguments: // lcl - lcl to replace with value if profitable