From d10b877d542b13924151aad39ca927882162111e Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 17 Dec 2020 09:08:37 -0800 Subject: [PATCH] JIT: introduce redundant branch opt phase Move the redundant branch elimination out of assertion prop into its own phase, in anticipation of future enhancements. Run it a bit earlier (before CSE). --- src/coreclr/jit/CMakeLists.txt | 1 + src/coreclr/jit/assertionprop.cpp | 152 ---------------- src/coreclr/jit/compiler.cpp | 7 + src/coreclr/jit/compiler.h | 5 + src/coreclr/jit/compphases.h | 1 + src/coreclr/jit/jitconfigvalues.h | 1 + src/coreclr/jit/redundantbranchopts.cpp | 221 ++++++++++++++++++++++++ 7 files changed, 236 insertions(+), 152 deletions(-) create mode 100644 src/coreclr/jit/redundantbranchopts.cpp diff --git a/src/coreclr/jit/CMakeLists.txt b/src/coreclr/jit/CMakeLists.txt index a2776acc38ae51..e5ebed0952b7c5 100644 --- a/src/coreclr/jit/CMakeLists.txt +++ b/src/coreclr/jit/CMakeLists.txt @@ -121,6 +121,7 @@ set( JIT_SOURCES phase.cpp rangecheck.cpp rationalize.cpp + redundantbranchopts.cpp regalloc.cpp register_arg_convention.cpp regset.cpp diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index ba172be4c5bd9a..53fd692601dca2 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3211,7 +3211,6 @@ GenTree* Compiler::optAssertionProp_RelOp(ASSERT_VALARG_TP assertions, GenTree* //------------------------------------------------------------------------ // optAssertionProp: try and optimize a relop via assertion propagation -// (and dominator based redundant branch elimination) // // Arguments: // assertions - set of live assertions @@ -3221,163 +3220,12 @@ GenTree* Compiler::optAssertionProp_RelOp(ASSERT_VALARG_TP assertions, GenTree* // Returns: // The modified tree, or nullptr if no assertion prop took place. // -// Notes: -// Redundant branch elimination doesn't rely on assertions currently, -// it is done here out of convenience. -// GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt) { GenTree* newTree = tree; GenTree* op1 = tree->AsOp()->gtOp1; GenTree* op2 = tree->AsOp()->gtOp2; - // First, walk up the dom tree and see if any dominating block has branched on - // exactly this tree's VN... - // - BasicBlock* prevBlock = compCurBB; - BasicBlock* domBlock = compCurBB->bbIDom; - int relopValue = -1; - - while (domBlock != nullptr) - { - if (prevBlock == compCurBB) - { - JITDUMP("\nVN relop, checking " FMT_BB " for redundancy\n", compCurBB->bbNum); - } - - // Check the current dominator - // - JITDUMP(" ... checking dom " FMT_BB "\n", domBlock->bbNum); - - if (domBlock->bbJumpKind == BBJ_COND) - { - Statement* const domJumpStmt = domBlock->lastStmt(); - GenTree* const domJumpTree = domJumpStmt->GetRootNode(); - assert(domJumpTree->OperIs(GT_JTRUE)); - GenTree* const domCmpTree = domJumpTree->AsOp()->gtGetOp1(); - - if (domCmpTree->OperKind() & GTK_RELOP) - { - ValueNum domCmpVN = domCmpTree->GetVN(VNK_Conservative); - - // Note we could also infer the tree relop's value from similar relops higher in the dom tree. - // For example, (x >= 0) dominating (x > 0), or (x < 0) dominating (x > 0). - // - // That is left as a future enhancement. - // - if (domCmpVN == tree->GetVN(VNK_Conservative)) - { - // Thes compare in "tree" is redundant. - // Is there a unique path from the dominating compare? - JITDUMP(" Redundant compare; current relop:\n"); - DISPTREE(tree); - JITDUMP(" dominating relop in " FMT_BB " with same VN:\n", domBlock->bbNum); - DISPTREE(domCmpTree); - - BasicBlock* trueSuccessor = domBlock->bbJumpDest; - BasicBlock* falseSuccessor = domBlock->bbNext; - - const bool trueReaches = fgReachable(trueSuccessor, compCurBB); - const bool falseReaches = fgReachable(falseSuccessor, compCurBB); - - if (trueReaches && falseReaches) - { - // Both dominating compare outcomes reach the current block so we can't infer the - // value of the relop. - // - // If the dominating compare is close to the current compare, this may be a missed - // opportunity to tail duplicate. - // - JITDUMP("Both successors of " FMT_BB " reach, can't optimize\n", domBlock->bbNum); - - if ((trueSuccessor->GetUniqueSucc() == compCurBB) || - (falseSuccessor->GetUniqueSucc() == compCurBB)) - { - JITDUMP("Perhaps we should have tail duplicated " FMT_BB "\n", compCurBB->bbNum); - } - } - else if (trueReaches) - { - // Taken jump in dominator reaches, fall through doesn't; relop must be true. - // - JITDUMP("Jump successor " FMT_BB " of " FMT_BB " reaches, relop must be true\n", - domBlock->bbJumpDest->bbNum, domBlock->bbNum); - relopValue = 1; - break; - } - else if (falseReaches) - { - // Fall through from dominator reaches, taken jump doesn't; relop must be false. - // - JITDUMP("Fall through successor " FMT_BB " of " FMT_BB " reaches, relop must be false\n", - domBlock->bbNext->bbNum, domBlock->bbNum); - relopValue = 0; - break; - } - else - { - // No apparent path from the dominating BB. - // - // If domBlock or compCurBB is in an EH handler we may fail to find a path. - // Just ignore those cases. - // - // No point in looking further up the tree. - // - break; - } - } - } - } - - // Keep looking higher up in the tree - // - prevBlock = domBlock; - domBlock = domBlock->bbIDom; - } - - // Did we determine the relop value via dominance checks? If so, optimize. - // - if (relopValue == -1) - { - JITDUMP("Failed to find a suitable dominating compare, so we won't optimize\n"); - } - else - { - // Bail out if tree is has certain side effects - // - // Note we really shouldn't get here if the tree has non-exception effects, - // as they should have impacted the value number. - // - if ((tree->gtFlags & GTF_SIDE_EFFECT) != 0) - { - // Bail if there is a non-exception effect. - // - if ((tree->gtFlags & GTF_SIDE_EFFECT) != GTF_EXCEPT) - { - JITDUMP("Current relop has non-exception side effects, so we won't optimize\n"); - return nullptr; - } - - // Be conservative if there is an exception effect and we're in an EH region - // as we might not model the full extent of EH flow. - // - if (compCurBB->hasTryIndex()) - { - JITDUMP("Current relop has exception side effect and is in a try, so we won't optimize\n"); - return nullptr; - } - } - - JITDUMP("\nVN relop based redundant branch opt in " FMT_BB ":\n", compCurBB->bbNum); - - tree->ChangeOperConst(GT_CNS_INT); - tree->AsIntCon()->gtIconVal = relopValue; - - newTree = fgMorphTree(tree); - DISPTREE(newTree); - return optAssertionProp_Update(newTree, tree, stmt); - } - // Look for assertions of the form (tree EQ/NE 0) AssertionIndex index = optGlobalAssertionIsEqualOrNotEqualZero(assertions, tree); diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 171bebb45e72b6..b25afff5a6fdaa 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -4826,6 +4826,7 @@ void Compiler::compCompile(void** methodCodePtr, ULONG* methodCodeSize, JitFlags bool doValueNum = true; bool doLoopHoisting = true; bool doCopyProp = true; + bool doBranchOpt = true; bool doAssertionProp = true; bool doRangeAnalysis = true; int iterations = 1; @@ -4836,6 +4837,7 @@ void Compiler::compCompile(void** methodCodePtr, ULONG* methodCodeSize, JitFlags doValueNum = doSsa && (JitConfig.JitDoValueNumber() != 0); doLoopHoisting = doValueNum && (JitConfig.JitDoLoopHoisting() != 0); doCopyProp = doValueNum && (JitConfig.JitDoCopyProp() != 0); + doBranchOpt = doValueNum && (JitConfig.JitDoRedundantBranchOpts() != 0); doAssertionProp = doValueNum && (JitConfig.JitDoAssertionProp() != 0); doRangeAnalysis = doAssertionProp && (JitConfig.JitDoRangeAnalysis() != 0); @@ -4882,6 +4884,11 @@ void Compiler::compCompile(void** methodCodePtr, ULONG* methodCodeSize, JitFlags DoPhase(this, PHASE_VN_COPY_PROP, &Compiler::optVnCopyProp); } + if (doBranchOpt) + { + DoPhase(this, PHASE_OPTIMIZE_BRANCHES, &Compiler::optRedundantBranches); + } + #if FEATURE_ANYCSE // Remove common sub-expressions // diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 5c3cdd9218d514..6539b755a45c09 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -6828,6 +6828,11 @@ class Compiler BasicBlock* basicBlock); #endif + // Redundant branch opts + // + PhaseStatus optRedundantBranches(); + bool optRedundantBranch(BasicBlock* const block); + #if ASSERTION_PROP /************************************************************************** * Value/Assertion propagation diff --git a/src/coreclr/jit/compphases.h b/src/coreclr/jit/compphases.h index 50e5afad7b5a23..4f304f3c363ab6 100644 --- a/src/coreclr/jit/compphases.h +++ b/src/coreclr/jit/compphases.h @@ -80,6 +80,7 @@ CompPhaseNameMacro(PHASE_OPTIMIZE_VALNUM_CSES, "Optimize Valnum CSEs", #endif CompPhaseNameMacro(PHASE_VN_COPY_PROP, "VN based copy prop", "CP-PROP", false, -1, false) +CompPhaseNameMacro(PHASE_OPTIMIZE_BRANCHES, "Redundant branch opts", "OPT-BR", false, -1, false) #if ASSERTION_PROP CompPhaseNameMacro(PHASE_ASSERTION_PROP_MAIN, "Assertion prop", "AST-PROP", false, -1, false) #endif diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index 7b14eb30bfdc4e..865ae3033f09aa 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/src/coreclr/jit/jitconfigvalues.h @@ -342,6 +342,7 @@ CONFIG_INTEGER(JitDoCopyProp, W("JitDoCopyProp"), 1) // Perform copy propagati CONFIG_INTEGER(JitDoEarlyProp, W("JitDoEarlyProp"), 1) // Perform Early Value Propagation CONFIG_INTEGER(JitDoLoopHoisting, W("JitDoLoopHoisting"), 1) // Perform loop hoisting on loop invariant values CONFIG_INTEGER(JitDoRangeAnalysis, W("JitDoRangeAnalysis"), 1) // Perform range check analysis +CONFIG_INTEGER(JitDoRedundantBranchOpts, W("JitDoRedundantBranchOpts"), 1) // Perform redundant branch optimizations CONFIG_INTEGER(JitDoSsa, W("JitDoSsa"), 1) // Perform Static Single Assignment (SSA) numbering on the variables CONFIG_INTEGER(JitDoValueNumber, W("JitDoValueNumber"), 1) // Perform value numbering on method expressions diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp new file mode 100644 index 00000000000000..f7584905cd2443 --- /dev/null +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -0,0 +1,221 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#include "jitpch.h" + +//------------------------------------------------------------------------ +// optRedundantBranches: try and optimize redundant branches in the method +// +// Returns: +// PhaseStatus indicating if anything changed. +// +PhaseStatus Compiler::optRedundantBranches() +{ + // We attempt this "bottom up" so walk the flow graph in postorder. + // + bool madeChanges = false; + + for (unsigned i = fgDomBBcount; i > 0; --i) + { + BasicBlock* const block = fgBBInvPostOrder[i]; + + // Upstream phases like optOptimizeBools may remove blocks + // that are referenced in bbInvPosOrder. + // + if ((block->bbFlags & BBF_REMOVED) != 0) + { + continue; + } + + // We currently can optimize some BBJ_CONDs. + // + if (block->bbJumpKind == BBJ_COND) + { + madeChanges |= optRedundantBranch(block); + } + } + + return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; +} + +//------------------------------------------------------------------------ +// optRedundantBranch: try and optimize a possibly redundant branch +// +// Arguments: +// block - block with branch to optimize +// +// Returns: +// True if the branch was optimized. +// +bool Compiler::optRedundantBranch(BasicBlock* const block) +{ + Statement* const stmt = block->lastStmt(); + + if (stmt == nullptr) + { + return false; + } + + GenTree* const jumpTree = stmt->GetRootNode(); + + if (!jumpTree->OperIs(GT_JTRUE)) + { + return false; + } + + GenTree* const tree = jumpTree->AsOp()->gtOp1; + + if (!(tree->OperKind() & GTK_RELOP)) + { + return false; + } + + // Walk up the dom tree and see if any dominating block has branched on + // exactly this tree's VN... + // + BasicBlock* prevBlock = block; + BasicBlock* domBlock = block->bbIDom; + int relopValue = -1; + + if (domBlock == nullptr) + { + return false; + } + + while (domBlock != nullptr) + { + if (prevBlock == block) + { + JITDUMP("\nChecking " FMT_BB " for redundancy\n", block->bbNum); + } + + // Check the current dominator + // + JITDUMP(" ... checking dom " FMT_BB "\n", domBlock->bbNum); + + if (domBlock->bbJumpKind == BBJ_COND) + { + Statement* const domJumpStmt = domBlock->lastStmt(); + GenTree* const domJumpTree = domJumpStmt->GetRootNode(); + assert(domJumpTree->OperIs(GT_JTRUE)); + GenTree* const domCmpTree = domJumpTree->AsOp()->gtGetOp1(); + + if (domCmpTree->OperKind() & GTK_RELOP) + { + ValueNum domCmpVN = domCmpTree->GetVN(VNK_Conservative); + + // Note we could also infer the tree relop's value from similar relops higher in the dom tree. + // For example, (x >= 0) dominating (x > 0), or (x < 0) dominating (x > 0). + // + // That is left as a future enhancement. + // + if (domCmpVN == tree->GetVN(VNK_Conservative)) + { + // Thes compare in "tree" is redundant. + // Is there a unique path from the dominating compare? + JITDUMP(" Redundant compare; current relop:\n"); + DISPTREE(tree); + JITDUMP(" dominating relop in " FMT_BB " with same VN:\n", domBlock->bbNum); + DISPTREE(domCmpTree); + + BasicBlock* trueSuccessor = domBlock->bbJumpDest; + BasicBlock* falseSuccessor = domBlock->bbNext; + + const bool trueReaches = fgReachable(trueSuccessor, block); + const bool falseReaches = fgReachable(falseSuccessor, block); + + if (trueReaches && falseReaches) + { + // Both dominating compare outcomes reach the current block so we can't infer the + // value of the relop. + // + // If the dominating compare is close to the current compare, this may be a missed + // opportunity to tail duplicate. + // + JITDUMP("Both successors of " FMT_BB " reach, can't optimize\n", domBlock->bbNum); + + if ((trueSuccessor->GetUniqueSucc() == block) || (falseSuccessor->GetUniqueSucc() == block)) + { + JITDUMP("Perhaps we should have tail duplicated " FMT_BB "\n", block->bbNum); + } + } + else if (trueReaches) + { + // Taken jump in dominator reaches, fall through doesn't; relop must be true. + // + JITDUMP("Jump successor " FMT_BB " of " FMT_BB " reaches, relop must be true\n", + domBlock->bbJumpDest->bbNum, domBlock->bbNum); + relopValue = 1; + break; + } + else if (falseReaches) + { + // Fall through from dominator reaches, taken jump doesn't; relop must be false. + // + JITDUMP("Fall through successor " FMT_BB " of " FMT_BB " reaches, relop must be false\n", + domBlock->bbNext->bbNum, domBlock->bbNum); + relopValue = 0; + break; + } + else + { + // No apparent path from the dominating BB. + // + // If domBlock or block is in an EH handler we may fail to find a path. + // Just ignore those cases. + // + // No point in looking further up the tree. + // + break; + } + } + } + } + + // Keep looking higher up in the tree + // + prevBlock = domBlock; + domBlock = domBlock->bbIDom; + } + + // Did we determine the relop value via dominance checks? If so, optimize. + // + if (relopValue == -1) + { + JITDUMP("Failed to find a suitable dominating compare, so we won't optimize\n"); + return false; + } + + // Bail out if tree is has certain side effects + // + // Note we really shouldn't get here if the tree has non-exception effects, + // as they should have impacted the value number. + // + if ((tree->gtFlags & GTF_SIDE_EFFECT) != 0) + { + // Bail if there is a non-exception effect. + // + if ((tree->gtFlags & GTF_SIDE_EFFECT) != GTF_EXCEPT) + { + JITDUMP("Current relop has non-exception side effects, so we won't optimize\n"); + return false; + } + + // Be conservative if there is an exception effect and we're in an EH region + // as we might not model the full extent of EH flow. + // + if (block->hasTryIndex()) + { + JITDUMP("Current relop has exception side effect and is in a try, so we won't optimize\n"); + return false; + } + } + + JITDUMP("\nRedundant branch opt in " FMT_BB ":\n", block->bbNum); + + tree->ChangeOperConst(GT_CNS_INT); + tree->AsIntCon()->gtIconVal = relopValue; + + fgMorphBlockStmt(block, stmt DEBUGARG(__FUNCTION__)); + return true; +}