diff options
author | Joseph Tremoulet <jotrem@microsoft.com> | 2016-07-18 12:56:37 -0400 |
---|---|---|
committer | Joseph Tremoulet <jotrem@microsoft.com> | 2016-08-02 17:21:37 -0400 |
commit | 16c65845eef41548c5653f14fede5a8c5c0be30f (patch) | |
tree | 2f90f82dc6753eb9147380b0b4202f39b58ec655 /src | |
parent | d5030420e022efde193ec53de4d9c98252f32fc9 (diff) | |
download | coreclr-16c65845eef41548c5653f14fede5a8c5c0be30f.tar.gz coreclr-16c65845eef41548c5653f14fede5a8c5c0be30f.tar.bz2 coreclr-16c65845eef41548c5653f14fede5a8c5c0be30f.zip |
Optimize bounds checks with correct assertion set
Assertion prop defers removing redundant bounds checks until processing
the parent comma, because the rewrite involves the comma as well. The
code currently has a bug in that its check for whether the bounds check
is redundant also occurs when processing reaches the comma, which is wrong
because the assertion set may then include assertions generated by the RHS
of the comma (there is code to compensate for the assertion generated by
the bounds check itself, which is the LHS of the comma, but no compensation
for the RHS).
This change moves the analysis of whether bounds checks are redundant to
the proper spot in the processing order, and delays only the rewrite to
the processing of the comma; the redundancy of the bounds check is
communicated via a new opcode-specific flag: GTF_ARR_BOUND_INBND.
Fixes #6318.
Diffstat (limited to 'src')
-rw-r--r-- | src/jit/assertionprop.cpp | 47 | ||||
-rw-r--r-- | src/jit/gentree.h | 2 |
2 files changed, 19 insertions, 30 deletions
diff --git a/src/jit/assertionprop.cpp b/src/jit/assertionprop.cpp index 004e7d9903..750e2e30db 100644 --- a/src/jit/assertionprop.cpp +++ b/src/jit/assertionprop.cpp @@ -3227,25 +3227,13 @@ GenTreePtr Compiler::optAssertionProp_Comma(ASSERT_VALARG_TP assertions const GenTreePtr tree, const GenTreePtr stmt) { - // Process the bounds check as part of the GT_COMMA node since, we need parent pointer to remove nodes. - if (tree->gtGetOp1()->OperGet() == GT_ARR_BOUNDS_CHECK) - { - // Since the GT_COMMA tree gets processed by assertion prop after the child GT_ARR_BOUNDS_CHECK - // node in execution order, bounds check assertions will be included for the parent GT_COMMA node. - // Remove the assertion made by the bounds check tree about itself. Assertion only applies to - // "future" bounds checks. - AssertionIndex index = (AssertionIndex)tree->gtGetOp1()->GetAssertion(); - if (index != NO_ASSERTION_INDEX && optGetAssertion(index)->IsBoundsCheckNoThrow()) - { - BitVecOps::RemoveElemD(apTraits, assertions, index - 1); - GenTreePtr newTree = optAssertionProp_BndsChk(assertions, tree, stmt); - BitVecOps::AddElemD(apTraits, assertions, index - 1); - return newTree; - } - else - { - return optAssertionProp_BndsChk(assertions, tree, stmt); - } + // Remove the bounds check as part of the GT_COMMA node since we need parent pointer to remove nodes. + // When processing visits the bounds check, it sets the throw kind to None if the check is redundant. + if ((tree->gtGetOp1()->OperGet() == GT_ARR_BOUNDS_CHECK) + && ((tree->gtGetOp1()->gtFlags & GTF_ARR_BOUND_INBND) != 0)) + { + optRemoveRangeCheck(tree, stmt, true, GTF_ASG, true /* force remove */); + return optAssertionProp_Update(tree, tree, stmt); } return nullptr; } @@ -3518,7 +3506,7 @@ GenTreePtr Compiler::optAssertionProp_BndsChk(ASSERT_VALARG_TP assertions, const return nullptr; } - assert(tree->gtOper == GT_COMMA && tree->gtGetOp1()->OperGet() == GT_ARR_BOUNDS_CHECK); + assert(tree->gtOper == GT_ARR_BOUNDS_CHECK); BitVecOps::Iter iter(apTraits, assertions); unsigned index = 0; @@ -3533,7 +3521,7 @@ GenTreePtr Compiler::optAssertionProp_BndsChk(ASSERT_VALARG_TP assertions, const continue; } - GenTreeBoundsChk* arrBndsChk = tree->gtGetOp1()->AsBoundsChk(); + GenTreeBoundsChk* arrBndsChk = tree->AsBoundsChk(); // Set 'isRedundant' to true if we can determine that 'arrBndsChk' can be // classified as a redundant bounds check using 'curAssertion' @@ -3609,15 +3597,11 @@ GenTreePtr Compiler::optAssertionProp_BndsChk(ASSERT_VALARG_TP assertions, const gtDispTree(tree, 0, nullptr, true); } #endif - optRemoveRangeCheck(tree, stmt, true, GTF_ASG, true /* force remove */); - GenTreePtr newTree = optAssertionProp_Update(tree, tree, stmt); - if (newTree != nullptr) - { - BitVecOps::RemoveElemD(apTraits, assertions, index - 1); - newTree = optAssertionProp(assertions, tree, stmt); - BitVecOps::AddElemD(apTraits, assertions, index - 1); - return newTree; - } + + // Defer actually removing the tree until processing reaches its parent comma, since + // optRemoveRangeCheck needs to rewrite the whole comma tree. + arrBndsChk->gtFlags |= GTF_ARR_BOUND_INBND; + return nullptr; } return nullptr; } @@ -3703,6 +3687,9 @@ GenTreePtr Compiler::optAssertionProp(ASSERT_VALARG_TP assertions, case GT_NULLCHECK: return optAssertionProp_Ind(assertions, tree, stmt); + case GT_ARR_BOUNDS_CHECK: + return optAssertionProp_BndsChk(assertions, tree, stmt); + case GT_COMMA: return optAssertionProp_Comma(assertions, tree, stmt); diff --git a/src/jit/gentree.h b/src/jit/gentree.h index 99ff508bdd..dd02d91464 100644 --- a/src/jit/gentree.h +++ b/src/jit/gentree.h @@ -844,6 +844,8 @@ public: #define GTF_NO_OP_NO 0x80000000 // GT_NO_OP --Have the codegenerator generate a special nop + #define GTF_ARR_BOUND_INBND 0x80000000 // GT_ARR_BOUNDS_CHECK -- have proved this check is always in-bounds + //---------------------------------------------------------------- #define GTF_STMT_CMPADD 0x80000000 // GT_STMT -- added by compiler |