From c1af69a7504f61cb0b0fbe343743b8472ae7f087 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 6 Nov 2022 02:34:48 +0100 Subject: [PATCH 1/5] attempt #3 --- src/coreclr/jit/assertionprop.cpp | 4 ++-- src/coreclr/scripts/superpmi_diffs.py | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 9a4be3c6e4e9e3..2313556ff2c14f 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3275,7 +3275,7 @@ bool Compiler::optIsProfitableToSubstitute(GenTreeLclVarCommon* lcl, BasicBlock* gtPrepareCost(value); - if ((value->GetCostEx() > 1) && (value->GetCostSz() > 1)) + if ((value->GetCostEx() > 2) && (value->GetCostSz() > 1)) { // Try to find the block this constant was originally defined in if (lcl->HasSsaName()) @@ -3289,7 +3289,7 @@ bool Compiler::optIsProfitableToSubstitute(GenTreeLclVarCommon* lcl, BasicBlock* const weight_t defBlockWeight = defBlock->getBBWeight(this); const weight_t lclblockWeight = lclBlock->getBBWeight(this); - if ((defBlockWeight > 0) && ((lclblockWeight / defBlockWeight) >= BB_LOOP_WEIGHT_SCALE)) + if ((defBlockWeight > 0) && ((lclblockWeight / defBlockWeight) >= 4)) { JITDUMP("Constant propagation inside loop " FMT_BB " is not profitable\n", lclBlock->bbNum); return false; diff --git a/src/coreclr/scripts/superpmi_diffs.py b/src/coreclr/scripts/superpmi_diffs.py index aaedd05155377c..16aa288dc6872b 100644 --- a/src/coreclr/scripts/superpmi_diffs.py +++ b/src/coreclr/scripts/superpmi_diffs.py @@ -207,6 +207,7 @@ def main(main_args): os.path.join(script_dir, "superpmi.py"), "asmdiffs", "--no_progress", + "-metrics", "PerfScore", "-core_root", core_root_dir, "-target_os", platform_name, "-target_arch", arch_name, From 00c24aefc7287a01a92d47544af5707befa6f477 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 6 Nov 2022 13:17:41 +0100 Subject: [PATCH 2/5] Scan multiple defs --- src/coreclr/jit/assertionprop.cpp | 47 +++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 2313556ff2c14f..269a5052e67e04 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3275,26 +3275,51 @@ bool Compiler::optIsProfitableToSubstitute(GenTreeLclVarCommon* lcl, BasicBlock* gtPrepareCost(value); - if ((value->GetCostEx() > 2) && (value->GetCostSz() > 1)) + 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); + auto isBlockWeightDifferenceTooBig = [](Compiler* comp, BasicBlock* defBlock, BasicBlock* useBlock) -> bool { + 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(comp); + const weight_t lclblockWeight = useBlock->getBBWeight(comp); - if ((defBlockWeight > 0) && ((lclblockWeight / defBlockWeight) >= 4)) + if ((defBlockWeight > 0) && ((lclblockWeight / defBlockWeight) >= 4)) + { + return true; + } + } + return false; + }; + + // Scan up to 4 defs to see if we have a def with much lower block's weight + GenTreeLclVarCommon* currlcl = lcl; + for (size_t i = 0; i < 4; i++) + { + LclSsaVarDsc* def = lvaGetDesc(currlcl)->GetPerSsaData(currlcl->GetSsaNum()); + if (isBlockWeightDifferenceTooBig(this, def->GetBlock(), lclBlock)) { - JITDUMP("Constant propagation inside loop " FMT_BB " is not profitable\n", lclBlock->bbNum); return false; } + GenTreeOp* asgNode = def->GetAssignment(); + if (asgNode != nullptr) + { + GenTree* val = asgNode->gtGetOp2(); + if (val->IsLocal()) + { + currlcl = val->AsLclVarCommon(); + continue; + } + } + + return true; } + } } return true; From b6dbb7025f537f2c642403d5dd936fa0a303c50c Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Sun, 6 Nov 2022 13:29:52 +0100 Subject: [PATCH 3/5] Update assertionprop.cpp --- src/coreclr/jit/assertionprop.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 269a5052e67e04..c7c8512732b2e0 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3302,6 +3302,10 @@ bool Compiler::optIsProfitableToSubstitute(GenTreeLclVarCommon* lcl, BasicBlock* for (size_t i = 0; i < 4; i++) { LclSsaVarDsc* def = lvaGetDesc(currlcl)->GetPerSsaData(currlcl->GetSsaNum()); + if (def == nullptr) + { + return true; + } if (isBlockWeightDifferenceTooBig(this, def->GetBlock(), lclBlock)) { return false; From a8e965635d71691f963bbeb488313dbbeef80ad9 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 6 Nov 2022 15:40:25 +0100 Subject: [PATCH 4/5] ignore complex asgs --- src/coreclr/jit/assertionprop.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index c7c8512732b2e0..cff47b220b0633 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3313,10 +3313,11 @@ bool Compiler::optIsProfitableToSubstitute(GenTreeLclVarCommon* lcl, BasicBlock* GenTreeOp* asgNode = def->GetAssignment(); if (asgNode != nullptr) { - GenTree* val = asgNode->gtGetOp2(); - if (val->IsLocal()) + GenTree* lhs = asgNode->gtGetOp1(); + GenTree* rhs = asgNode->gtGetOp2(); + if (rhs->IsLocal() && lhs->IsLocal() && lhs->AsLclVarCommon()->GetLclNum() == currlcl->GetLclNum()) { - currlcl = val->AsLclVarCommon(); + currlcl = rhs->AsLclVarCommon(); continue; } } From 2727f5ad4f6674ccba7f5dae0f3a9c688d8957a8 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Mon, 7 Nov 2022 02:25:43 +0100 Subject: [PATCH 5/5] Update assertionprop.cpp --- src/coreclr/jit/assertionprop.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index cff47b220b0633..116edfa0a96574 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3301,6 +3301,10 @@ bool Compiler::optIsProfitableToSubstitute(GenTreeLclVarCommon* lcl, BasicBlock* GenTreeLclVarCommon* currlcl = lcl; for (size_t i = 0; i < 4; i++) { + if (currlcl->GetSsaNum() == SsaConfig::RESERVED_SSA_NUM) + { + return true; + } LclSsaVarDsc* def = lvaGetDesc(currlcl)->GetPerSsaData(currlcl->GetSsaNum()); if (def == nullptr) {