From 31eaf3266da60e3172c09dc71208c3e4cf77cbbb Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Mon, 22 Feb 2021 10:23:39 -0800 Subject: [PATCH 1/3] fix assertion merge for degenerate flow --- src/coreclr/jit/assertionprop.cpp | 25 ++++++++++++++++++++----- src/coreclr/jit/dataflow.h | 4 ++-- src/coreclr/jit/morph.cpp | 6 +++--- src/coreclr/jit/optcse.cpp | 2 +- 4 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 53fd692601dca2..48de7ce1a6db0f 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4648,14 +4648,29 @@ class AssertionPropFlowCallback } // During merge, perform the actual merging of the predecessor's (since this is a forward analysis) dataflow flags. - void Merge(BasicBlock* block, BasicBlock* predBlock, flowList* preds) + void Merge(BasicBlock* block, BasicBlock* predBlock, unsigned dupCount) { - ASSERT_TP pAssertionOut = ((predBlock->bbJumpKind == BBJ_COND) && (predBlock->bbJumpDest == block)) - ? mJumpDestOut[predBlock->bbNum] - : predBlock->bbAssertionOut; + ASSERT_TP pAssertionOut; + if (dupCount == 2) + { + assert((predBlock->bbJumpKind == BBJ_COND) && (predBlock->bbJumpDest == block) && + (predBlock->bbNext == block)); + pAssertionOut = mJumpDestOut[predBlock->bbNum]; + BitVecOps::IntersectionD(apTraits, pAssertionOut, predBlock->bbAssertionOut); + } + else if ((predBlock->bbJumpKind == BBJ_COND) && (predBlock->bbJumpDest == block)) + { + pAssertionOut = mJumpDestOut[predBlock->bbNum]; + } + else + { + assert(dupCount == 1); + pAssertionOut = predBlock->bbAssertionOut; + } + JITDUMP("AssertionPropCallback::Merge : " FMT_BB " in -> %s, predBlock " FMT_BB " out -> %s\n", block->bbNum, BitVecOps::ToString(apTraits, block->bbAssertionIn), predBlock->bbNum, - BitVecOps::ToString(apTraits, predBlock->bbAssertionOut)); + BitVecOps::ToString(apTraits, pAssertionOut)); BitVecOps::IntersectionD(apTraits, block->bbAssertionIn, pAssertionOut); } diff --git a/src/coreclr/jit/dataflow.h b/src/coreclr/jit/dataflow.h index 3f2ff1cc72a24c..7e84642dce692b 100644 --- a/src/coreclr/jit/dataflow.h +++ b/src/coreclr/jit/dataflow.h @@ -16,7 +16,7 @@ // { // public: // void StartMerge(BasicBlock* block); -// void Merge(BasicBlock* block, BasicBlock* pred, flowList* preds); +// void Merge(BasicBlock* block, BasicBlock* pred, unsigned dupCount); // bool EndMerge(BasicBlock* block); // }; #pragma once @@ -61,7 +61,7 @@ void DataFlow::ForwardAnalysis(TCallback& callback) flowList* preds = m_pCompiler->BlockPredsWithEH(block); for (flowList* pred = preds; pred; pred = pred->flNext) { - callback.Merge(block, pred->getBlock(), preds); + callback.Merge(block, pred->getBlock(), pred->flDupCount); } } diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 639129a38bad68..31f9a518965c4a 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -16245,9 +16245,9 @@ bool Compiler::fgFoldConditional(BasicBlock* block) // no side effects, remove the jump entirely fgRemoveStmt(block, lastStmt); } - // block is a BBJ_COND that we are folding the conditional for - // bTaken is the path that will always be taken from block - // bNotTaken is the path that will never be taken from block + // block is a BBJ_COND that we are folding the conditional for. + // bTaken is the path that will always be taken from block. + // bNotTaken is the path that will never be taken from block. // BasicBlock* bTaken; BasicBlock* bNotTaken; diff --git a/src/coreclr/jit/optcse.cpp b/src/coreclr/jit/optcse.cpp index 83a86207c06a60..181b62b150cb8a 100644 --- a/src/coreclr/jit/optcse.cpp +++ b/src/coreclr/jit/optcse.cpp @@ -1188,7 +1188,7 @@ class CSE_DataFlow } // Merge: perform the merging of each of the predecessor's liveness values (since this is a forward analysis) - void Merge(BasicBlock* block, BasicBlock* predBlock, flowList* preds) + void Merge(BasicBlock* block, BasicBlock* predBlock, unsigned dupCount) { #ifdef DEBUG if (m_comp->verbose) From 06fe7343ad81783e6388ebb1874b24b9c1c64fbc Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Mon, 22 Feb 2021 12:25:50 -0800 Subject: [PATCH 2/3] fix the dupCount condition --- src/coreclr/jit/assertionprop.cpp | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 48de7ce1a6db0f..e2bfb86ea0ac7f 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4651,20 +4651,19 @@ class AssertionPropFlowCallback void Merge(BasicBlock* block, BasicBlock* predBlock, unsigned dupCount) { ASSERT_TP pAssertionOut; - if (dupCount == 2) - { - assert((predBlock->bbJumpKind == BBJ_COND) && (predBlock->bbJumpDest == block) && - (predBlock->bbNext == block)); - pAssertionOut = mJumpDestOut[predBlock->bbNum]; - BitVecOps::IntersectionD(apTraits, pAssertionOut, predBlock->bbAssertionOut); - } - else if ((predBlock->bbJumpKind == BBJ_COND) && (predBlock->bbJumpDest == block)) + + if (predBlock->bbJumpKind == BBJ_COND && (predBlock->bbJumpDest == block)) { pAssertionOut = mJumpDestOut[predBlock->bbNum]; + + if (dupCount > 1) + { + assert(predBlock->bbNext == block); + BitVecOps::IntersectionD(apTraits, pAssertionOut, predBlock->bbAssertionOut); + } } else { - assert(dupCount == 1); pAssertionOut = predBlock->bbAssertionOut; } From 481f04751b4e53d02f2b10f49e6a750911603bc6 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Wed, 24 Feb 2021 00:00:15 -0800 Subject: [PATCH 3/3] Add comment and jitdump --- src/coreclr/jit/assertionprop.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index e2bfb86ea0ac7f..5d105881933fa5 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4658,8 +4658,16 @@ class AssertionPropFlowCallback if (dupCount > 1) { + // Scenario where next block and conditional block, both point to the same block. + // In such case, intersect the assertions present on both the out edges of predBlock. assert(predBlock->bbNext == block); BitVecOps::IntersectionD(apTraits, pAssertionOut, predBlock->bbAssertionOut); + + JITDUMP("AssertionPropCallback::Merge : Duplicate flow, " FMT_BB " in -> %s, predBlock " FMT_BB + " out1 -> %s, out2 -> %s\n", + block->bbNum, BitVecOps::ToString(apTraits, block->bbAssertionIn), predBlock->bbNum, + BitVecOps::ToString(apTraits, mJumpDestOut[predBlock->bbNum]), + BitVecOps::ToString(apTraits, predBlock->bbAssertionOut)); } } else