--- old/src/share/vm/opto/loopnode.hpp 2014-09-17 18:42:42.492067888 +0200 +++ new/src/share/vm/opto/loopnode.hpp 2014-09-17 18:42:42.314932270 +0200 @@ -602,6 +602,8 @@ return ctrl; } + bool insert_castii_before_loop(Node* incr, Node* ctrl, Node* loop); + public: bool has_node( Node* n ) const { guarantee(n != NULL, "No Node."); --- /dev/null 2014-06-17 14:40:17.837204769 +0200 +++ new/test/compiler/loopopts/TestDeadBackbranchArrayAccess.java 2014-09-17 18:42:42.339403224 +0200 @@ -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-09-17 18:42:42.556540187 +0200 +++ new/src/share/vm/opto/loopTransform.cpp 2014-09-17 18:42:42.357182866 +0200 @@ -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,21 @@ return n; } +bool PhaseIdealLoop::insert_castii_before_loop(Node* incr, Node* ctrl, Node* loop) { + Node* castii = new CastIINode(incr, TypeInt::INT, true, true); + castii->set_req(0, ctrl); + castii = _igvn.transform(castii); + assert(castii->Opcode() == Op_CastII, "required CastII cannot go away"); + 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 +1097,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 = insert_castii_before_loop(zer_opaq->in(1), zer_taken, post_head); + assert(inserted, "no castII inserted"); + + // CastII for the main loop: + inserted = insert_castii_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-09-17 18:42:42.573371559 +0200 +++ new/src/share/vm/opto/castnode.hpp 2014-09-17 18:42:42.381075552 +0200 @@ -48,10 +48,23 @@ //------------------------------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 _required; + // Is the type of this CastII the best we can do or do we expect to + // improve it later on and so we shouldn't optimize it out? + const bool _inexact; public: - CastIINode (Node *n, const Type *t ): ConstraintCastNode(n,t) {} + CastIINode(Node *n, const Type *t, bool required = false, bool inexact = false) + : ConstraintCastNode(n,t), _required(required), _inexact(inexact) {} virtual int Opcode() const; virtual uint ideal_reg() const { return Op_RegI; } + virtual Node *Identity( PhaseTransform *phase ); + virtual Node *Ideal(PhaseGVN *phase, bool can_reshape); + virtual Node *Ideal_DU_postCCP( PhaseCCP * ); +#ifndef PRODUCT + virtual void dump_spec(outputStream *st) const; +#endif }; //------------------------------CastPPNode------------------------------------- --- old/src/share/vm/opto/opaquenode.hpp 2014-09-17 18:42:42.633045671 +0200 +++ new/src/share/vm/opto/opaquenode.hpp 2014-09-17 18:42:42.406370399 +0200 @@ -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------------------------------------ --- old/src/share/vm/opto/opaquenode.cpp 2014-09-17 18:42:42.678546000 +0200 +++ new/src/share/vm/opto/opaquenode.cpp 2014-09-17 18:42:42.431912042 +0200 @@ -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,51 @@ // 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 (cmp->in(1)->Opcode() == Op_AddI) { + Node *add = cmp->in(1); + if (add->in(1)->is_Phi()) { + Node* phi = add->in(1); + if (phi->in(0)->is_CountedLoop()) { + CountedLoopNode* cl = phi->in(0)->as_CountedLoop(); + if (cl->phi() == phi) { + // 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. + phase->is_IterGVN()->_worklist.push(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-09-17 18:42:42.677579956 +0200 +++ new/src/share/vm/opto/castnode.cpp 2014-09-17 18:42:42.451626249 +0200 @@ -83,6 +83,95 @@ return this; } +Node *CastIINode::Identity(PhaseTransform *phase) { + if (_inexact) { + return this; + } + return ConstraintCastNode::Identity(phase); +} + +Node *CastIINode::Ideal(PhaseGVN *phase, bool can_reshape) { + Node* res = ConstraintCastNode::Ideal(phase, can_reshape); + // Try to improve the type of the CastII if we recognize a CmpI/If + // pattern. + if (res == NULL && _inexact) { + 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)) { + 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 { + ShouldNotReachHere(); + } + 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); + t = t->filter_speculative(_type); + if (_type != t || in2_t->is_con()) { + CastIINode* castii = new CastIINode(in(1), t, _required, !in2_t->is_con()); + castii->set_req(0, in(0)); + res = castii; + } + } + } + } + } + } + return res; +} + +Node *CastIINode::Ideal_DU_postCCP(PhaseCCP *ccp) { + if (_required) { + return NULL; + } + return ConstraintCastNode::Ideal_DU_postCCP(ccp); +} + +#ifndef PRODUCT +void CastIINode::dump_spec(outputStream *st) const { + TypeNode::dump_spec(st); + if (_inexact) { + st->print(" inexact"); + } + if (_required) { + st->print(" required"); + } +} +#endif //=============================================================================