--- old/src/share/vm/opto/opaquenode.hpp 2014-11-07 22:46:15.478069076 +0100 +++ new/src/share/vm/opto/opaquenode.hpp 2014-11-07 22:46:15.304789740 +0100 @@ -50,7 +50,8 @@ Node* original_loop_limit() { return req()==3 ? in(2) : NULL; } virtual int Opcode() const; virtual const Type *bottom_type() const { return TypeInt::INT; } - virtual Node *Identity( PhaseTransform *phase ); + virtual Node *Identity(PhaseTransform *phase); + virtual Node *Ideal(PhaseGVN *phase, bool can_reshape); }; //------------------------------Opaque2Node------------------------------------ --- /dev/null 2014-10-17 05:25:30.601014041 +0200 +++ new/test/compiler/loopopts/TestDeadBackbranchArrayAccess.java 2014-11-07 22:46:15.394335783 +0100 @@ -0,0 +1,58 @@ +/* + * Copyright (c) 2014, 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 8054478 + * @summary dead backbranch in main loop results in erroneous array access + * @run main/othervm -XX:CompileOnly=TestDeadBackbranchArrayAccess -Xcomp TestDeadBackbranchArrayAccess + * + */ + +public class TestDeadBackbranchArrayAccess { + static char[] pattern0 = {0}; + static char[] pattern1 = {1}; + + static void test(char[] array) { + if (pattern1 == null) return; + + int i = 0; + int pos = 0; + char c = array[pos]; + + while (i >= 0 && (c == pattern0[i] || c == pattern1[i])) { + i--; + pos--; + if (pos != -1) { + c = array[pos]; + } + } + } + + public static void main(String[] args) { + for (int i = 0; i < 1000000; i++) { + test(new char[1]); + } + } +} --- old/src/share/vm/opto/loopTransform.cpp 2014-11-07 22:46:15.500764438 +0100 +++ new/src/share/vm/opto/loopTransform.cpp 2014-11-07 22:46:15.326656808 +0100 @@ -27,6 +27,7 @@ #include "memory/allocation.inline.hpp" #include "opto/addnode.hpp" #include "opto/callnode.hpp" +#include "opto/castnode.hpp" #include "opto/connode.hpp" #include "opto/convertnode.hpp" #include "opto/divnode.hpp" @@ -885,6 +886,20 @@ return n; } +bool PhaseIdealLoop::cast_incr_before_loop(Node* incr, Node* ctrl, Node* loop) { + Node* castii = new CastIINode(incr, TypeInt::INT, true); + castii->set_req(0, ctrl); + register_new_node(castii, ctrl); + for (DUIterator_Fast imax, i = incr->fast_outs(imax); i < imax; i++) { + Node* n = incr->fast_out(i); + if (n->is_Phi() && n->in(0) == loop) { + int nrep = n->replace_edge(incr, castii); + return true; + } + } + return false; +} + //------------------------------insert_pre_post_loops-------------------------- // Insert pre and post loops. If peel_only is set, the pre-loop can not have // more iterations added. It acts as a 'peel' only, no lower-bound RCE, no @@ -1081,6 +1096,24 @@ } } + // Nodes inside the loop may be control dependent on a predicate + // that was moved before the preloop. If the back branch of the main + // or post loops becomes dead, those nodes won't be dependent on the + // test that guards that loop nest anymore which could lead to an + // incorrect array access because it executes independently of the + // test that was guarding the loop nest. We add a special CastII on + // the if branch that enters the loop, between the input induction + // variable value and the induction variable Phi to preserve correct + // dependencies. + + // CastII for the post loop: + bool inserted = cast_incr_before_loop(zer_opaq->in(1), zer_taken, post_head); + assert(inserted, "no castII inserted"); + + // CastII for the main loop: + inserted = cast_incr_before_loop(pre_incr, min_taken, main_head); + assert(inserted, "no castII inserted"); + // Step B4: Shorten the pre-loop to run only 1 iteration (for now). // RCE and alignment may change this later. Node *cmp_end = pre_end->cmp_node(); --- old/src/share/vm/opto/castnode.hpp 2014-11-07 22:46:15.531690515 +0100 +++ new/src/share/vm/opto/castnode.hpp 2014-11-07 22:46:15.352759949 +0100 @@ -48,10 +48,25 @@ //------------------------------CastIINode------------------------------------- // cast integer to integer (different range) class CastIINode: public ConstraintCastNode { + private: + // Can this node be removed post CCP or does it carry a required dependency? + const bool _carry_dependency; + + protected: + virtual uint cmp( const Node &n ) const; + virtual uint size_of() const; + public: - CastIINode (Node *n, const Type *t ): ConstraintCastNode(n,t) {} + CastIINode(Node *n, const Type *t, bool carry_dependency = false) + : ConstraintCastNode(n,t), _carry_dependency(carry_dependency) {} virtual int Opcode() const; virtual uint ideal_reg() const { return Op_RegI; } + virtual Node *Identity( PhaseTransform *phase ); + virtual const Type *Value( PhaseTransform *phase ) const; + virtual Node *Ideal_DU_postCCP( PhaseCCP * ); +#ifndef PRODUCT + virtual void dump_spec(outputStream *st) const; +#endif }; //------------------------------CastPPNode------------------------------------- --- old/src/share/vm/opto/opaquenode.cpp 2014-11-07 22:46:15.564707002 +0100 +++ new/src/share/vm/opto/opaquenode.cpp 2014-11-07 22:46:15.371356409 +0100 @@ -23,13 +23,14 @@ */ #include "precompiled.hpp" +#include "opto/loopnode.hpp" #include "opto/opaquenode.hpp" #include "opto/phaseX.hpp" //============================================================================= // Do not allow value-numbering uint Opaque1Node::hash() const { return NO_HASH; } -uint Opaque1Node::cmp( const Node &n ) const { +uint Opaque1Node::cmp(const Node &n) const { return (&n == this); // Always fail except on self } @@ -40,10 +41,48 @@ // call to IterGVN and any chance of hitting this code. Hence there's no // phase-ordering problem with stripping Opaque1 in IGVN followed by some // more loop optimizations that require it. -Node *Opaque1Node::Identity( PhaseTransform *phase ) { +Node *Opaque1Node::Identity(PhaseTransform *phase) { return phase->C->major_progress() ? this : in(1); } +Node *Opaque1Node::Ideal(PhaseGVN *phase, bool can_reshape) { + if (!phase->C->major_progress() && outcnt() > 0) { + assert(can_reshape, "loop opts over but not IGVN?"); + if (raw_out(0)->Opcode() == Op_CmpI) { + Node* cmp = raw_out(0); + // If this opaque node feeds into the limit condition of a + // CountedLoop, we need to process the Phi node for the + // induction variable: the range of values taken by the Phi is + // known now and so its type is also known. + if (cmp->raw_out(0)->is_Bool()) { + Node* b = cmp->raw_out(0); + if (b->raw_out(0)->is_CountedLoopEnd()) { + CountedLoopEndNode* cle = b->raw_out(0)->as_CountedLoopEnd(); + if (cle->limit() == this) { + phase->is_IterGVN()->_worklist.push(cle->phi()); + } + } + } + Node* in1 = cmp->in(1); + for (uint i = 0; i < in1->outcnt(); i++) { + if (in1->raw_out(i)->Opcode() == Op_CastII) { + Node* castii = in1->raw_out(i); + if (castii->in(0) != NULL && castii->in(0)->in(0) != NULL && castii->in(0)->in(0)->is_If()) { + Node* ifnode = castii->in(0)->in(0); + if (ifnode->in(1) != NULL && ifnode->in(1)->in(1) == cmp) { + // Reprocess a CastII node that may depend on this + // opaque node value in case it's inexact and we can do + // a better job of setting its type. + phase->is_IterGVN()->_worklist.push(castii); + } + } + } + } + } + } + return NULL; +} + //============================================================================= // A node to prevent unwanted optimizations. Allows constant folding. Stops // value-numbering, most Ideal calls or Identity functions. This Node is --- old/src/share/vm/opto/castnode.cpp 2014-11-07 22:46:15.639560116 +0100 +++ new/src/share/vm/opto/castnode.cpp 2014-11-07 22:46:15.413207888 +0100 @@ -83,6 +83,101 @@ return this; } +uint CastIINode::size_of() const { + return sizeof(*this); +} + +uint CastIINode::cmp(const Node &n) const { + return TypeNode::cmp(n) && ((CastIINode&)n)._carry_dependency == _carry_dependency; +} + +Node *CastIINode::Identity(PhaseTransform *phase) { + if (_carry_dependency) { + return this; + } + return ConstraintCastNode::Identity(phase); +} + +const Type *CastIINode::Value(PhaseTransform *phase) const { + const Type *res = ConstraintCastNode::Value(phase); + + // Try to improve the type of the CastII if we recognize a CmpI/If + // pattern. + if (_carry_dependency) { + if (in(0) != NULL && (in(0)->is_IfFalse() || in(0)->is_IfTrue())) { + Node* proj = in(0); + if (proj->in(0)->in(1)->is_Bool()) { + Node* b = proj->in(0)->in(1); + if (b->in(1)->Opcode() == Op_CmpI) { + Node* cmp = b->in(1); + if (cmp->in(1) == in(1) && phase->type(cmp->in(2))->isa_int()) { + const TypeInt* in2_t = phase->type(cmp->in(2))->is_int(); + const Type* t = TypeInt::INT; + BoolTest test = b->as_Bool()->_test; + if (proj->is_IfFalse()) { + test = test.negate(); + } + BoolTest::mask m = test._test; + jlong lo_long = min_jint; + jlong hi_long = max_jint; + if (m == BoolTest::le || m == BoolTest::lt) { + hi_long = in2_t->_hi; + if (m == BoolTest::lt) { + hi_long -= 1; + } + } else if (m == BoolTest::ge || m == BoolTest::gt) { + lo_long = in2_t->_lo; + if (m == BoolTest::gt) { + lo_long += 1; + } + } else if (m == BoolTest::eq) { + lo_long = in2_t->_lo; + hi_long = in2_t->_hi; + } else if (m == BoolTest::ne) { + // can't do any better + } else { + stringStream ss; + test.dump_on(&ss); + fatal(err_msg_res("unexpected comparison %s", ss.as_string())); + } + int lo_int = (int)lo_long; + int hi_int = (int)hi_long; + + if (lo_long != (jlong)lo_int) { + lo_int = min_jint; + } + if (hi_long != (jlong)hi_int) { + hi_int = max_jint; + } + + t = TypeInt::make(lo_int, hi_int, Type::WidenMax); + + res = res->filter_speculative(t); + + return res; + } + } + } + } + } + return res; +} + +Node *CastIINode::Ideal_DU_postCCP(PhaseCCP *ccp) { + if (_carry_dependency) { + return NULL; + } + return ConstraintCastNode::Ideal_DU_postCCP(ccp); +} + +#ifndef PRODUCT +void CastIINode::dump_spec(outputStream *st) const { + TypeNode::dump_spec(st); + if (_carry_dependency) { + st->print(" carry dependency"); + } +} +#endif //============================================================================= --- old/src/share/vm/opto/loopnode.hpp 2014-11-07 22:46:15.640671145 +0100 +++ new/src/share/vm/opto/loopnode.hpp 2014-11-07 22:46:15.436987493 +0100 @@ -602,6 +602,8 @@ return ctrl; } + bool cast_incr_before_loop(Node* incr, Node* ctrl, Node* loop); + public: bool has_node( Node* n ) const { guarantee(n != NULL, "No Node.");