8177095: Range check dependent CastII/ConvI2L is prematurely eliminated

Wed, 29 Mar 2017 09:20:08 +0200

author
thartmann
date
Wed, 29 Mar 2017 09:20:08 +0200
changeset 8727
3d8d14307428
parent 8725
60d621df6c58
child 8728
8119c543f2af

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.

mercurial