# HG changeset patch # User thartmann # Date 1490772008 -7200 # Node ID 69946653069110a57f9d49e40a05147c574966ca # Parent 79dd5370ad0fee8754546283326d972367bce5d6 8177095: Range check dependent CastII/ConvI2L is prematurely eliminated Summary: Disabled narrowing of range check dependent CastIIs (either through the CastII(AddI) optimization or through CastIINode::Ideal). Reviewed-by: vlivanov, kvn diff -r 79dd5370ad0f -r 699466530691 src/share/vm/opto/connode.cpp --- a/src/share/vm/opto/connode.cpp Tue May 24 18:42:01 2016 +0300 +++ b/src/share/vm/opto/connode.cpp Wed Mar 29 09:20:08 2017 +0200 @@ -979,8 +979,7 @@ } #ifdef _LP64 - // Convert ConvI2L(AddI(x, y)) to AddL(ConvI2L(x), ConvI2L(y)) or - // ConvI2L(CastII(AddI(x, y))) to AddL(ConvI2L(CastII(x)), ConvI2L(CastII(y))), + // Convert ConvI2L(AddI(x, y)) to AddL(ConvI2L(x), ConvI2L(y)) // but only if x and y have subranges that cannot cause 32-bit overflow, // under the assumption that x+y is in my own subrange this->type(). @@ -1004,13 +1003,6 @@ Node* z = in(1); int op = z->Opcode(); - Node* ctrl = NULL; - if (op == Op_CastII && z->as_CastII()->has_range_check()) { - // Skip CastII node but save control dependency - ctrl = z->in(0); - z = z->in(1); - op = z->Opcode(); - } if (op == Op_AddI || op == Op_SubI) { Node* x = z->in(1); Node* y = z->in(2); @@ -1070,8 +1062,8 @@ } assert(rxlo == (int)rxlo && rxhi == (int)rxhi, "x should not overflow"); assert(rylo == (int)rylo && ryhi == (int)ryhi, "y should not overflow"); - Node* cx = phase->C->constrained_convI2L(phase, x, TypeInt::make(rxlo, rxhi, widen), ctrl); - Node* cy = phase->C->constrained_convI2L(phase, y, TypeInt::make(rylo, ryhi, widen), ctrl); + Node* cx = phase->C->constrained_convI2L(phase, x, TypeInt::make(rxlo, rxhi, widen), NULL); + Node* cy = phase->C->constrained_convI2L(phase, y, TypeInt::make(rylo, ryhi, widen), NULL); switch (op) { case Op_AddI: return new (phase->C) AddLNode(cx, cy); case Op_SubI: return new (phase->C) SubLNode(cx, cy); diff -r 79dd5370ad0f -r 699466530691 test/compiler/loopopts/TestLoopPeeling.java --- a/test/compiler/loopopts/TestLoopPeeling.java Tue May 24 18:42:01 2016 +0300 +++ b/test/compiler/loopopts/TestLoopPeeling.java Wed Mar 29 09:20:08 2017 +0200 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2017, 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 @@ -23,10 +23,16 @@ /* * @test - * @bug 8078262 + * @bug 8078262 8177095 * @summary Tests correct dominator information after loop peeling. - * @run main/othervm -Xcomp -XX:CompileCommand=compileonly,TestLoopPeeling::test* TestLoopPeeling + * + * @run main/othervm -Xcomp + * -XX:CompileCommand=compileonly,compiler.loopopts.TestLoopPeeling::test* + * compiler.loopopts.TestLoopPeeling */ + +package compiler.loopopts; + public class TestLoopPeeling { public int[] array = new int[100]; @@ -34,14 +40,16 @@ public static void main(String args[]) { TestLoopPeeling test = new TestLoopPeeling(); try { - test.testArrayAccess(0, 1); + test.testArrayAccess1(0, 1); + test.testArrayAccess2(0); + test.testArrayAccess3(0, false); test.testArrayAllocation(0, 1); } catch (Exception e) { // Ignore exceptions } } - public void testArrayAccess(int index, int inc) { + public void testArrayAccess1(int index, int inc) { int storeIndex = -1; for (; index < 10; index += inc) { @@ -57,7 +65,7 @@ if (index == 42) { // This store and the corresponding range check are moved out of the - // loop and both used after old loop and the peeled iteration exit. + // loop and both used after main loop and the peeled iteration exit. // For the peeled iteration, storeIndex is always -1 and the ConvI2L // is replaced by TOP. However, the range check is not folded because // we don't do the split if optimization in PhaseIdealLoop2. @@ -71,6 +79,44 @@ } } + public int testArrayAccess2(int index) { + // Load1 and the corresponding range check are moved out of the loop + // and both are used after the main loop and the peeled iteration exit. + // For the peeled iteration, storeIndex is always Integer.MIN_VALUE and + // for the main loop it is 0. Hence, the merging phi has type int:<=0. + // Load1 reads the array at index ConvI2L(CastII(AddI(storeIndex, -1))) + // where the CastII is range check dependent and has type int:>=0. + // The CastII gets pushed through the AddI and its type is changed to int:>=1 + // which does not overlap with the input type of storeIndex (int:<=0). + // The CastII is replaced by TOP causing a cascade of other eliminations. + // Since the control path through the range check CmpU(AddI(storeIndex, -1)) + // is not eliminated, the graph is in a corrupted state. We fail once we merge + // with the result of Load2 because we get data from a non-dominating region. + int storeIndex = Integer.MIN_VALUE; + for (; index < 10; ++index) { + if (index == 42) { + return array[storeIndex-1]; // Load1 + } + storeIndex = 0; + } + return array[42]; // Load2 + } + + public int testArrayAccess3(int index, boolean b) { + // Same as testArrayAccess2 but manifests as crash in register allocator. + int storeIndex = Integer.MIN_VALUE; + for (; index < 10; ++index) { + if (b) { + return 0; + } + if (index == 42) { + return array[storeIndex-1]; // Load1 + } + storeIndex = 0; + } + return array[42]; // Load2 + } + public byte[] testArrayAllocation(int index, int inc) { int allocationCount = -1; byte[] result; @@ -82,7 +128,7 @@ if (index == 42) { // This allocation and the corresponding size check are moved out of the - // loop and both used after old loop and the peeled iteration exit. + // loop and both used after main loop and the peeled iteration exit. // For the peeled iteration, allocationCount is always -1 and the ConvI2L // is replaced by TOP. However, the size check is not folded because // we don't do the split if optimization in PhaseIdealLoop2.