From 86503785cd6414b8692e5c83cadaa2972b6a099b Mon Sep 17 00:00:00 2001 From: Roland Levillain Date: Thu, 11 Feb 2016 19:07:30 +0000 Subject: [PATCH] Fix x86-64 Baker's read barrier fast path for CheckCast. Use an art::x86_64::Label instead of an art::x86_64::NearLabel as end label when emitting code for a HCheckCast instruction, as the range of the latter may sometimes be too short when Baker's read barriers are enabled. Bug: 12687968 Change-Id: Ia9742dce65be7d4fb104688f3c4717b65df1fb54 --- compiler/optimizing/code_generator_x86_64.cc | 75 ++++++++++++++++++---- compiler/optimizing/graph_visualizer.cc | 2 + compiler/optimizing/nodes.cc | 22 +++++++ compiler/optimizing/nodes.h | 2 + test/573-checker-checkcast-regression/expected.txt | 1 + test/573-checker-checkcast-regression/info.txt | 4 ++ .../573-checker-checkcast-regression/src/Main.java | 49 ++++++++++++++ 7 files changed, 144 insertions(+), 11 deletions(-) create mode 100644 test/573-checker-checkcast-regression/expected.txt create mode 100644 test/573-checker-checkcast-regression/info.txt create mode 100644 test/573-checker-checkcast-regression/src/Main.java diff --git a/compiler/optimizing/code_generator_x86_64.cc b/compiler/optimizing/code_generator_x86_64.cc index 99396cd98..4f0f5f0ff 100644 --- a/compiler/optimizing/code_generator_x86_64.cc +++ b/compiler/optimizing/code_generator_x86_64.cc @@ -5840,19 +5840,20 @@ void InstructionCodeGeneratorX86_64::VisitCheckCast(HCheckCast* instruction) { is_type_check_slow_path_fatal); codegen_->AddSlowPath(type_check_slow_path); - NearLabel done; - // Avoid null check if we know obj is not null. - if (instruction->MustDoNullCheck()) { - __ testl(obj, obj); - __ j(kEqual, &done); - } - - // /* HeapReference */ temp = obj->klass_ - GenerateReferenceLoadTwoRegisters(instruction, temp_loc, obj_loc, class_offset, maybe_temp2_loc); - switch (type_check_kind) { case TypeCheckKind::kExactCheck: case TypeCheckKind::kArrayCheck: { + NearLabel done; + // Avoid null check if we know obj is not null. + if (instruction->MustDoNullCheck()) { + __ testl(obj, obj); + __ j(kEqual, &done); + } + + // /* HeapReference */ temp = obj->klass_ + GenerateReferenceLoadTwoRegisters( + instruction, temp_loc, obj_loc, class_offset, maybe_temp2_loc); + if (cls.IsRegister()) { __ cmpl(temp, cls.AsRegister()); } else { @@ -5862,10 +5863,22 @@ void InstructionCodeGeneratorX86_64::VisitCheckCast(HCheckCast* instruction) { // Jump to slow path for throwing the exception or doing a // more involved array check. __ j(kNotEqual, type_check_slow_path->GetEntryLabel()); + __ Bind(&done); break; } case TypeCheckKind::kAbstractClassCheck: { + NearLabel done; + // Avoid null check if we know obj is not null. + if (instruction->MustDoNullCheck()) { + __ testl(obj, obj); + __ j(kEqual, &done); + } + + // /* HeapReference */ temp = obj->klass_ + GenerateReferenceLoadTwoRegisters( + instruction, temp_loc, obj_loc, class_offset, maybe_temp2_loc); + // If the class is abstract, we eagerly fetch the super class of the // object to avoid doing a comparison we know will fail. NearLabel loop, compare_classes; @@ -5896,10 +5909,22 @@ void InstructionCodeGeneratorX86_64::VisitCheckCast(HCheckCast* instruction) { __ cmpl(temp, Address(CpuRegister(RSP), cls.GetStackIndex())); } __ j(kNotEqual, &loop); + __ Bind(&done); break; } case TypeCheckKind::kClassHierarchyCheck: { + NearLabel done; + // Avoid null check if we know obj is not null. + if (instruction->MustDoNullCheck()) { + __ testl(obj, obj); + __ j(kEqual, &done); + } + + // /* HeapReference */ temp = obj->klass_ + GenerateReferenceLoadTwoRegisters( + instruction, temp_loc, obj_loc, class_offset, maybe_temp2_loc); + // Walk over the class hierarchy to find a match. NearLabel loop; __ Bind(&loop); @@ -5927,10 +5952,26 @@ void InstructionCodeGeneratorX86_64::VisitCheckCast(HCheckCast* instruction) { GenerateReferenceLoadTwoRegisters( instruction, temp_loc, obj_loc, class_offset, maybe_temp2_loc); __ jmp(type_check_slow_path->GetEntryLabel()); + __ Bind(&done); break; } case TypeCheckKind::kArrayObjectCheck: { + // We cannot use a NearLabel here, as its range might be too + // short in some cases when read barriers are enabled. This has + // been observed for instance when the code emitted for this + // case uses high x86-64 registers (R8-R15). + Label done; + // Avoid null check if we know obj is not null. + if (instruction->MustDoNullCheck()) { + __ testl(obj, obj); + __ j(kEqual, &done); + } + + // /* HeapReference */ temp = obj->klass_ + GenerateReferenceLoadTwoRegisters( + instruction, temp_loc, obj_loc, class_offset, maybe_temp2_loc); + // Do an exact check. NearLabel check_non_primitive_component_type; if (cls.IsRegister()) { @@ -5969,11 +6010,23 @@ void InstructionCodeGeneratorX86_64::VisitCheckCast(HCheckCast* instruction) { GenerateReferenceLoadTwoRegisters( instruction, temp_loc, obj_loc, class_offset, maybe_temp2_loc); __ jmp(type_check_slow_path->GetEntryLabel()); + __ Bind(&done); break; } case TypeCheckKind::kUnresolvedCheck: case TypeCheckKind::kInterfaceCheck: + NearLabel done; + // Avoid null check if we know obj is not null. + if (instruction->MustDoNullCheck()) { + __ testl(obj, obj); + __ j(kEqual, &done); + } + + // /* HeapReference */ temp = obj->klass_ + GenerateReferenceLoadTwoRegisters( + instruction, temp_loc, obj_loc, class_offset, maybe_temp2_loc); + // We always go into the type check slow path for the unresolved // and interface check cases. // @@ -5992,9 +6045,9 @@ void InstructionCodeGeneratorX86_64::VisitCheckCast(HCheckCast* instruction) { // call to the runtime not using a type checking slow path). // This should also be beneficial for the other cases above. __ jmp(type_check_slow_path->GetEntryLabel()); + __ Bind(&done); break; } - __ Bind(&done); __ Bind(type_check_slow_path->GetExitLabel()); } diff --git a/compiler/optimizing/graph_visualizer.cc b/compiler/optimizing/graph_visualizer.cc index 9d796c100..63b8fd05c 100644 --- a/compiler/optimizing/graph_visualizer.cc +++ b/compiler/optimizing/graph_visualizer.cc @@ -368,11 +368,13 @@ class HGraphVisualizerPrinter : public HGraphDelegateVisitor { } void VisitCheckCast(HCheckCast* check_cast) OVERRIDE { + StartAttributeStream("check_kind") << check_cast->GetTypeCheckKind(); StartAttributeStream("must_do_null_check") << std::boolalpha << check_cast->MustDoNullCheck() << std::noboolalpha; } void VisitInstanceOf(HInstanceOf* instance_of) OVERRIDE { + StartAttributeStream("check_kind") << instance_of->GetTypeCheckKind(); StartAttributeStream("must_do_null_check") << std::boolalpha << instance_of->MustDoNullCheck() << std::noboolalpha; } diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc index f26988590..453571451 100644 --- a/compiler/optimizing/nodes.cc +++ b/compiler/optimizing/nodes.cc @@ -2387,4 +2387,26 @@ std::ostream& operator<<(std::ostream& os, const MoveOperands& rhs) { return os; } +std::ostream& operator<<(std::ostream& os, TypeCheckKind rhs) { + switch (rhs) { + case TypeCheckKind::kUnresolvedCheck: + return os << "unresolved_check"; + case TypeCheckKind::kExactCheck: + return os << "exact_check"; + case TypeCheckKind::kClassHierarchyCheck: + return os << "class_hierarchy_check"; + case TypeCheckKind::kAbstractClassCheck: + return os << "abstract_class_check"; + case TypeCheckKind::kInterfaceCheck: + return os << "interface_check"; + case TypeCheckKind::kArrayObjectCheck: + return os << "array_object_check"; + case TypeCheckKind::kArrayCheck: + return os << "array_check"; + default: + LOG(FATAL) << "Unknown TypeCheckKind: " << static_cast(rhs); + UNREACHABLE(); + } +} + } // namespace art diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h index daec096f3..7f463a36e 100644 --- a/compiler/optimizing/nodes.h +++ b/compiler/optimizing/nodes.h @@ -5451,6 +5451,8 @@ enum class TypeCheckKind { kArrayCheck // No optimization yet when checking against a generic array. }; +std::ostream& operator<<(std::ostream& os, TypeCheckKind rhs); + class HInstanceOf : public HExpression<2> { public: HInstanceOf(HInstruction* object, diff --git a/test/573-checker-checkcast-regression/expected.txt b/test/573-checker-checkcast-regression/expected.txt new file mode 100644 index 000000000..b8626c4cf --- /dev/null +++ b/test/573-checker-checkcast-regression/expected.txt @@ -0,0 +1 @@ +4 diff --git a/test/573-checker-checkcast-regression/info.txt b/test/573-checker-checkcast-regression/info.txt new file mode 100644 index 000000000..74a6d6ec0 --- /dev/null +++ b/test/573-checker-checkcast-regression/info.txt @@ -0,0 +1,4 @@ +Regression test for the x86-64 Baker's read barrier fast path compiler +instrumentation of CheckCasts, where we used to use an +art::x86_64::NearLabel, the range of which was sometimes too short +with Baker's read barriers enabled. diff --git a/test/573-checker-checkcast-regression/src/Main.java b/test/573-checker-checkcast-regression/src/Main.java new file mode 100644 index 000000000..473a2b164 --- /dev/null +++ b/test/573-checker-checkcast-regression/src/Main.java @@ -0,0 +1,49 @@ +/* + * Copyright (C) 2016 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 void main(String[] args) { + Object[] array = { new Integer(1), new Integer(2), new Integer(3) }; + int result = test(array, 0, 2); + System.out.println(result); + } + + // This test method uses two integers (`index1` and `index2`) to + // force the register allocator to use some high registers (R8-R15) + // on x86-64 in the code generated for the first CheckCast (which + // converts `new_array` to an `Object[]`), so as to produce code + // containing a conditional jump whose offset does not fit in a + // NearLabel when using Baker's read barrier fast path (because + // x86-64 instructions using these high registers have a larger + // encoding). + // + // The intent of this artifical constraint is to ensure the initial + // failure is properly tested by this regression test. + + /// CHECK-START: int Main.test(java.lang.Object, int, int) register (after) + /// CHECK-DAG: CheckCast check_kind:array_object_check + /// CHECK-DAG: CheckCast check_kind:exact_check + /// CHECK-DAG: CheckCast check_kind:exact_check + + static public int test(Object new_array, int index1, int index2) { + Object[] objectArray = (Object[]) new_array; + Integer integer1 = (Integer) objectArray[index1]; + Integer integer2 = (Integer) objectArray[index2]; + return integer1.intValue() + integer2.intValue(); + } + +} -- 2.11.0