--- old/src/share/vm/ci/ciTypeFlow.cpp 2015-10-30 14:46:18.795964489 +0100 +++ new/src/share/vm/ci/ciTypeFlow.cpp 2015-10-30 14:46:18.695964486 +0100 @@ -1588,6 +1588,7 @@ _exceptions = NULL; _exc_klasses = NULL; _successors = NULL; + _predecessors = new (outer->arena()) GrowableArray(outer->arena(), 1, 0, NULL); _state = new (outer->arena()) StateVector(outer); JsrSet* new_jsrs = new (outer->arena()) JsrSet(outer->arena(), jsrs->size()); @@ -1771,6 +1772,12 @@ break; } } + + // Set predecessor information + for (int i = 0; i < _successors->length(); i++) { + Block* block = _successors->at(i); + block->predecessors()->append(this); + } } return _successors; } @@ -1909,6 +1916,18 @@ st->cr(); } } + if (_predecessors == NULL) { + st->print_cr(" No predecessor information"); + } else { + int num_predecessors = _predecessors->length(); + st->print_cr(" Predecessors : %d", num_predecessors); + for (int i = 0; i < num_predecessors; i++) { + Block* predecessor = _predecessors->at(i); + st->print(" "); + predecessor->print_value_on(st); + st->cr(); + } + } if (_exceptions == NULL) { st->print_cr(" No exception information"); } else { @@ -2884,6 +2903,67 @@ } // ------------------------------------------------------------------ +// ciTypeFlow::is_dominated_by +// +// Determine if the instruction at bci is dominated by the instruction at dom_bci. +bool ciTypeFlow::is_dominated_by(int bci, int dom_bci) { + ResourceMark rm; + JsrSet* jsr_null = new ciTypeFlow::JsrSet(NULL); + int index = _methodBlocks->block_containing(bci)->index(); + int dom_index = _methodBlocks->block_containing(dom_bci)->index(); + Block* block = get_block_for(index, jsr_null, ciTypeFlow::no_create); + Block* dom_block = get_block_for(dom_index, jsr_null, ciTypeFlow::no_create); + + // Start block dominates all other blocks + if (start_block()->rpo() == dom_block->rpo()) { + return true; + } + + // Dominated[i] is true if block i is dominated by dom_block + int num_blocks = _methodBlocks->num_blocks(); + bool* dominated = NEW_RESOURCE_ARRAY(bool, num_blocks); + for (int i = 0; i < num_blocks; ++i) { + dominated[i] = true; + } + dominated[start_block()->rpo()] = false; + + // Iterative dominator algorithm + bool changed = true; + while (changed) { + changed = false; + // Use reverse postorder iteration + for (Block* blk = _rpo_list; blk != NULL; blk = blk->rpo_next()) { + if (blk->is_start()) { + // Ignore start block + continue; + } + // The block is dominated if it is the dominating block + // itself or if all predecessors are dominated. + int index = blk->rpo(); + bool dom = (index == dom_block->rpo()); + if (!dom) { + // Check if all predecessors are dominated + dom = true; + for (int i = 0; i < blk->predecessors()->length(); ++i) { + Block* pred = blk->predecessors()->at(i); + if (!dominated[pred->rpo()]) { + dom = false; + break; + } + } + } + // Update dominator information + if (dominated[index] != dom) { + changed = true; + dominated[index] = dom; + } + } + } + // block dominated by dom_block? + return dominated[block->rpo()]; +} + +// ------------------------------------------------------------------ // ciTypeFlow::record_failure() // The ciTypeFlow object keeps track of failure reasons separately from the ciEnv. // This is required because there is not a 1-1 relation between the ciEnv and --- old/src/share/vm/ci/ciTypeFlow.hpp 2015-10-30 14:46:19.075964498 +0100 +++ new/src/share/vm/ci/ciTypeFlow.hpp 2015-10-30 14:46:19.003964495 +0100 @@ -529,6 +529,7 @@ GrowableArray* _exceptions; GrowableArray* _exc_klasses; GrowableArray* _successors; + GrowableArray* _predecessors; StateVector* _state; JsrSet* _jsrs; @@ -617,6 +618,11 @@ return _successors; } + GrowableArray* predecessors() { + assert(_predecessors != NULL, "must be filled in"); + return _predecessors; + } + // Get the exceptional successors for this Block. GrowableArray* exceptions() { if (_exceptions == NULL) { @@ -941,6 +947,9 @@ // Perform type inference flow analysis. void do_flow(); + // Determine if bci is dominated by dom_bci + bool is_dominated_by(int bci, int dom_bci); + void print_on(outputStream* st) const PRODUCT_RETURN; void rpo_print_on(outputStream* st) const PRODUCT_RETURN; --- old/src/share/vm/opto/ifnode.cpp 2015-10-30 14:46:19.359964507 +0100 +++ new/src/share/vm/opto/ifnode.cpp 2015-10-30 14:46:19.287964504 +0100 @@ -23,6 +23,7 @@ */ #include "precompiled.hpp" +#include "ci/ciTypeFlow.hpp" #include "memory/allocation.inline.hpp" #include "opto/addnode.hpp" #include "opto/castnode.hpp" @@ -768,7 +769,8 @@ // Do this If and the dominating If both branch out to an uncommon trap bool IfNode::has_only_uncommon_traps(ProjNode* proj, ProjNode*& success, ProjNode*& fail, PhaseIterGVN* igvn) { ProjNode* otherproj = proj->other_if_proj(); - CallStaticJavaNode* dom_unc = otherproj->is_uncommon_trap_proj(Deoptimization::Reason_none); + // Dominating uncommon trap must have 'Reason_unstable_if' to ensure re-execution of folded Ifs + CallStaticJavaNode* dom_unc = otherproj->is_uncommon_trap_proj(Deoptimization::Reason_unstable_if); if (otherproj->outcnt() == 1 && dom_unc != NULL) { CallStaticJavaNode* unc = NULL; @@ -784,12 +786,28 @@ } else if (dom_unc->in(0) != otherproj || unc->in(0) != unc_proj) { return false; } + + // Check if the bci of the dominating uncommon trap dominates the bci + // of the dominated uncommon trap. Otherwise we may not re-execute + // the dominated check after deoptimization from the merged uncommon trap. + ciMethod* method = unc->jvms()->method(); + ciMethod* dom_method = dom_unc->jvms()->method(); + if (method != dom_method) { + return false; + } + ciTypeFlow* flow = dom_method->get_flow_analysis(); + int bci = unc->jvms()->bci(); + int dom_bci = dom_unc->jvms()->bci(); + if (!flow->is_dominated_by(bci, dom_bci)) { + return false; + } + // See merge_uncommon_traps: the reason of the uncommon trap // will be changed and the state of the dominating If will be // used. Checked that we didn't apply this transformation in a // previous compilation and it didn't cause too many traps - if (!igvn->C->too_many_traps(dom_unc->jvms()->method(), dom_unc->jvms()->bci(), Deoptimization::Reason_unstable_fused_if) && - !igvn->C->too_many_traps(dom_unc->jvms()->method(), dom_unc->jvms()->bci(), Deoptimization::Reason_range_check)) { + if (!igvn->C->too_many_traps(dom_method, dom_bci, Deoptimization::Reason_unstable_fused_if) && + !igvn->C->too_many_traps(dom_method, dom_bci, Deoptimization::Reason_range_check)) { success = unc_proj; fail = unc_proj->other_if_proj(); return true; --- /dev/null 2015-10-30 10:00:53.843909024 +0100 +++ new/test/compiler/rangechecks/TestUncommonTrapMerging.java 2015-10-30 14:46:19.531964512 +0100 @@ -0,0 +1,100 @@ +/* + * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8140574 + * @summary Verify proper re-execution of checks after merging of uncommon traps + * @run main/othervm -Xcomp -XX:-TieredCompilation -XX:CompileCommand=compileonly,TestUncommonTrapMerging::test* TestUncommonTrapMerging Test1 + * @run main/othervm -XX:CompileCommand=compileonly,TestUncommonTrapMerging::test* TestUncommonTrapMerging Test2 + */ +public class TestUncommonTrapMerging { + + public static void main(String[] args) throws Throwable { + if (args.length < 1) { + throw new RuntimeException("Not enough arguments!"); + } + TestUncommonTrapMerging mytest = new TestUncommonTrapMerging(); + String testcase = args[0]; + if (testcase.equals("Test1")) { + try { + // '42' should hit the 'arg > 0' check + mytest.test(42); + + } catch (OutOfMemoryError e) { + // expected + } + } else if (testcase.equals("Test2")) { + // Compile test2 with uncommon traps at path 1 and path 2 + for (int i = 0; i < 100_000; i++) { + mytest.test2(-1, 0); + } + + // Compile test3 which inlines test2 with uncommon traps at + // path 1 and path 2. Because test3 always passes 'value = 1', + // C2 will remove the 'value > 0' check and then merge the two + // uncommon traps. + for (int i = 0; i < 100_000; i++) { + mytest.test3(0); + } + + // This should return through path 2 + if (!mytest.test3(42)) { + throw new RuntimeException("test2 returned through wrong path!"); + } + } + } + + public void test(int arg) throws Throwable { + // The following two checks should not be merged if the + // uncommon trap of the dominating if has 'Reason_unloaded' + // because we need to re-execute both checks after deopt. + if (arg < 0) { + throw new RuntimeException("Should not reach here"); + } else if (arg > 0) { + throw new OutOfMemoryError(); + } + throw new RuntimeException("Should not reach here"); + } + + public boolean test2(int arg, int value) { + if (arg < 0) { + if (value > 0) { + // path 1 + return false; + } + } else if (arg > 0) { + // path 2 + return true; + } + // path 3 + return false; + } + + public boolean test3(int arg) { + int i; + for (i = 0; i < 1; ++i) { } + // i == 1 + return test2(arg, i); + } +}