OSDN Git Service

Fix x86-64 Baker's read barrier fast path for CheckCast.
authorRoland Levillain <rpl@google.com>
Thu, 11 Feb 2016 19:07:30 +0000 (19:07 +0000)
committerRoland Levillain <rpl@google.com>
Thu, 11 Feb 2016 19:07:30 +0000 (19:07 +0000)
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
compiler/optimizing/graph_visualizer.cc
compiler/optimizing/nodes.cc
compiler/optimizing/nodes.h
test/573-checker-checkcast-regression/expected.txt [new file with mode: 0644]
test/573-checker-checkcast-regression/info.txt [new file with mode: 0644]
test/573-checker-checkcast-regression/src/Main.java [new file with mode: 0644]

index 99396cd..4f0f5f0 100644 (file)
@@ -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<Class> */ 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<Class> */ temp = obj->klass_
+      GenerateReferenceLoadTwoRegisters(
+          instruction, temp_loc, obj_loc, class_offset, maybe_temp2_loc);
+
       if (cls.IsRegister()) {
         __ cmpl(temp, cls.AsRegister<CpuRegister>());
       } 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<Class> */ 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<Class> */ 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<Class> */ 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<Class> */ 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());
 }
index 9d796c1..63b8fd0 100644 (file)
@@ -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;
   }
index f269885..4535714 100644 (file)
@@ -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<int>(rhs);
+      UNREACHABLE();
+  }
+}
+
 }  // namespace art
index daec096..7f463a3 100644 (file)
@@ -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 (file)
index 0000000..b8626c4
--- /dev/null
@@ -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 (file)
index 0000000..74a6d6e
--- /dev/null
@@ -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 (file)
index 0000000..473a2b1
--- /dev/null
@@ -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();
+  }
+
+}