From 7d4cc8c786ff4a19234c1b034eae61ac0f3a37da Mon Sep 17 00:00:00 2001 From: Nicolas Geoffray Date: Sat, 20 Jun 2015 23:49:01 +0100 Subject: [PATCH] Fix wrong DCHECK in bounds check elimination. The lower range of an array length instruction can be changed by other instructions than HBoundsCheck, like HNewArray. bug:21862741 (cherry picked from commit 8d82a0c2b2b12f259ccb357d3b1e699c68ad0400) Change-Id: I1bb1a4f4c6673509dd3fb5184c32992bed876250 --- compiler/optimizing/bounds_check_elimination.cc | 4 ++- test/513-array-deopt/expected.txt | 0 test/513-array-deopt/info.txt | 2 ++ test/513-array-deopt/src/Main.java | 37 +++++++++++++++++++++++++ 4 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 test/513-array-deopt/expected.txt create mode 100644 test/513-array-deopt/info.txt create mode 100644 test/513-array-deopt/src/Main.java diff --git a/compiler/optimizing/bounds_check_elimination.cc b/compiler/optimizing/bounds_check_elimination.cc index 97b3725da..900dabea0 100644 --- a/compiler/optimizing/bounds_check_elimination.cc +++ b/compiler/optimizing/bounds_check_elimination.cc @@ -1759,7 +1759,9 @@ class BCEVisitor : public HGraphVisitor { ValueBound lower_bound = range->GetLower(); DCHECK(lower_bound.IsConstant()); DCHECK(const_instr->GetValue() <= kMaxConstantForAddingDeoptimize); - DCHECK_EQ(lower_bound.GetConstant(), const_instr->GetValue() + 1); + // Note that the lower bound of the array length may have been refined + // through other instructions (such as `HNewArray(length - 4)`). + DCHECK_LE(const_instr->GetValue() + 1, lower_bound.GetConstant()); // If array_length is less than lower_const, deoptimize. HBoundsCheck* bounds_check = first_constant_index_bounds_check_map_.Get( diff --git a/test/513-array-deopt/expected.txt b/test/513-array-deopt/expected.txt new file mode 100644 index 000000000..e69de29bb diff --git a/test/513-array-deopt/info.txt b/test/513-array-deopt/info.txt new file mode 100644 index 000000000..afce9d994 --- /dev/null +++ b/test/513-array-deopt/info.txt @@ -0,0 +1,2 @@ +Regression test for the BCE phase of optimizing, +that used to have wrong assumptions about array length bounds. diff --git a/test/513-array-deopt/src/Main.java b/test/513-array-deopt/src/Main.java new file mode 100644 index 000000000..a0ae4c355 --- /dev/null +++ b/test/513-array-deopt/src/Main.java @@ -0,0 +1,37 @@ +/* + * Copyright (C) 2015 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +public class Main { + public static int[] bar(int[] a) { + a[0] = 0; + a[1] = 0; + a[2] = 0; + // Up to this point, we record that the lower bound is 2. + // The next instruction will record that the lower bound is 5. + // The deoptimization code used to assume the lower bound has + // to be check it will add for the deoptimization (here, it + // would be 2). + return new int[a.length - 5]; + } + + public static void main(String[] args) { + int[] a = new int[5]; + a = bar(a); + if (a.length != 0) { + throw new Error("Expected 0, got " + a.length); + } + } +} -- 2.11.0