Wed, 29 Mar 2017 09:20:08 +0200
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
src/share/vm/opto/connode.cpp | file | annotate | diff | comparison | revisions | |
test/compiler/loopopts/TestLoopPeeling.java | file | annotate | diff | comparison | revisions |
1.1 --- a/src/share/vm/opto/connode.cpp Tue Mar 21 17:08:07 2017 -0700 1.2 +++ b/src/share/vm/opto/connode.cpp Wed Mar 29 09:20:08 2017 +0200 1.3 @@ -999,8 +999,7 @@ 1.4 } 1.5 1.6 #ifdef _LP64 1.7 - // Convert ConvI2L(AddI(x, y)) to AddL(ConvI2L(x), ConvI2L(y)) or 1.8 - // ConvI2L(CastII(AddI(x, y))) to AddL(ConvI2L(CastII(x)), ConvI2L(CastII(y))), 1.9 + // Convert ConvI2L(AddI(x, y)) to AddL(ConvI2L(x), ConvI2L(y)) 1.10 // but only if x and y have subranges that cannot cause 32-bit overflow, 1.11 // under the assumption that x+y is in my own subrange this->type(). 1.12 1.13 @@ -1024,13 +1023,6 @@ 1.14 1.15 Node* z = in(1); 1.16 int op = z->Opcode(); 1.17 - Node* ctrl = NULL; 1.18 - if (op == Op_CastII && z->as_CastII()->has_range_check()) { 1.19 - // Skip CastII node but save control dependency 1.20 - ctrl = z->in(0); 1.21 - z = z->in(1); 1.22 - op = z->Opcode(); 1.23 - } 1.24 if (op == Op_AddI || op == Op_SubI) { 1.25 Node* x = z->in(1); 1.26 Node* y = z->in(2); 1.27 @@ -1090,8 +1082,8 @@ 1.28 } 1.29 assert(rxlo == (int)rxlo && rxhi == (int)rxhi, "x should not overflow"); 1.30 assert(rylo == (int)rylo && ryhi == (int)ryhi, "y should not overflow"); 1.31 - Node* cx = phase->C->constrained_convI2L(phase, x, TypeInt::make(rxlo, rxhi, widen), ctrl); 1.32 - Node* cy = phase->C->constrained_convI2L(phase, y, TypeInt::make(rylo, ryhi, widen), ctrl); 1.33 + Node* cx = phase->C->constrained_convI2L(phase, x, TypeInt::make(rxlo, rxhi, widen), NULL); 1.34 + Node* cy = phase->C->constrained_convI2L(phase, y, TypeInt::make(rylo, ryhi, widen), NULL); 1.35 switch (op) { 1.36 case Op_AddI: return new (phase->C) AddLNode(cx, cy); 1.37 case Op_SubI: return new (phase->C) SubLNode(cx, cy);
2.1 --- a/test/compiler/loopopts/TestLoopPeeling.java Tue Mar 21 17:08:07 2017 -0700 2.2 +++ b/test/compiler/loopopts/TestLoopPeeling.java Wed Mar 29 09:20:08 2017 +0200 2.3 @@ -1,5 +1,5 @@ 2.4 /* 2.5 - * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved. 2.6 + * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved. 2.7 * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. 2.8 * 2.9 * This code is free software; you can redistribute it and/or modify it 2.10 @@ -23,10 +23,16 @@ 2.11 2.12 /* 2.13 * @test 2.14 - * @bug 8078262 2.15 + * @bug 8078262 8177095 2.16 * @summary Tests correct dominator information after loop peeling. 2.17 - * @run main/othervm -Xcomp -XX:CompileCommand=compileonly,TestLoopPeeling::test* TestLoopPeeling 2.18 + * 2.19 + * @run main/othervm -Xcomp 2.20 + * -XX:CompileCommand=compileonly,compiler.loopopts.TestLoopPeeling::test* 2.21 + * compiler.loopopts.TestLoopPeeling 2.22 */ 2.23 + 2.24 +package compiler.loopopts; 2.25 + 2.26 public class TestLoopPeeling { 2.27 2.28 public int[] array = new int[100]; 2.29 @@ -34,14 +40,16 @@ 2.30 public static void main(String args[]) { 2.31 TestLoopPeeling test = new TestLoopPeeling(); 2.32 try { 2.33 - test.testArrayAccess(0, 1); 2.34 + test.testArrayAccess1(0, 1); 2.35 + test.testArrayAccess2(0); 2.36 + test.testArrayAccess3(0, false); 2.37 test.testArrayAllocation(0, 1); 2.38 } catch (Exception e) { 2.39 // Ignore exceptions 2.40 } 2.41 } 2.42 2.43 - public void testArrayAccess(int index, int inc) { 2.44 + public void testArrayAccess1(int index, int inc) { 2.45 int storeIndex = -1; 2.46 2.47 for (; index < 10; index += inc) { 2.48 @@ -57,7 +65,7 @@ 2.49 2.50 if (index == 42) { 2.51 // This store and the corresponding range check are moved out of the 2.52 - // loop and both used after old loop and the peeled iteration exit. 2.53 + // loop and both used after main loop and the peeled iteration exit. 2.54 // For the peeled iteration, storeIndex is always -1 and the ConvI2L 2.55 // is replaced by TOP. However, the range check is not folded because 2.56 // we don't do the split if optimization in PhaseIdealLoop2. 2.57 @@ -71,6 +79,44 @@ 2.58 } 2.59 } 2.60 2.61 + public int testArrayAccess2(int index) { 2.62 + // Load1 and the corresponding range check are moved out of the loop 2.63 + // and both are used after the main loop and the peeled iteration exit. 2.64 + // For the peeled iteration, storeIndex is always Integer.MIN_VALUE and 2.65 + // for the main loop it is 0. Hence, the merging phi has type int:<=0. 2.66 + // Load1 reads the array at index ConvI2L(CastII(AddI(storeIndex, -1))) 2.67 + // where the CastII is range check dependent and has type int:>=0. 2.68 + // The CastII gets pushed through the AddI and its type is changed to int:>=1 2.69 + // which does not overlap with the input type of storeIndex (int:<=0). 2.70 + // The CastII is replaced by TOP causing a cascade of other eliminations. 2.71 + // Since the control path through the range check CmpU(AddI(storeIndex, -1)) 2.72 + // is not eliminated, the graph is in a corrupted state. We fail once we merge 2.73 + // with the result of Load2 because we get data from a non-dominating region. 2.74 + int storeIndex = Integer.MIN_VALUE; 2.75 + for (; index < 10; ++index) { 2.76 + if (index == 42) { 2.77 + return array[storeIndex-1]; // Load1 2.78 + } 2.79 + storeIndex = 0; 2.80 + } 2.81 + return array[42]; // Load2 2.82 + } 2.83 + 2.84 + public int testArrayAccess3(int index, boolean b) { 2.85 + // Same as testArrayAccess2 but manifests as crash in register allocator. 2.86 + int storeIndex = Integer.MIN_VALUE; 2.87 + for (; index < 10; ++index) { 2.88 + if (b) { 2.89 + return 0; 2.90 + } 2.91 + if (index == 42) { 2.92 + return array[storeIndex-1]; // Load1 2.93 + } 2.94 + storeIndex = 0; 2.95 + } 2.96 + return array[42]; // Load2 2.97 + } 2.98 + 2.99 public byte[] testArrayAllocation(int index, int inc) { 2.100 int allocationCount = -1; 2.101 byte[] result; 2.102 @@ -82,7 +128,7 @@ 2.103 2.104 if (index == 42) { 2.105 // This allocation and the corresponding size check are moved out of the 2.106 - // loop and both used after old loop and the peeled iteration exit. 2.107 + // loop and both used after main loop and the peeled iteration exit. 2.108 // For the peeled iteration, allocationCount is always -1 and the ConvI2L 2.109 // is replaced by TOP. However, the size check is not folded because 2.110 // we don't do the split if optimization in PhaseIdealLoop2.