changeset 1881:085521a03c8b

Forwardport Shark. 2009-06-06 Gary Benson <gbenson@redhat.com> * ports/hotspot/src/share/vm/shark/sharkBlock.hpp (SharkBlock::maybe_do_instanceof_if): New method. * ports/hotspot/src/share/vm/shark/sharkBlock.cpp (SharkBlock::maybe_do_instanceof_if): New method. (SharkBlock::parse_bytecode): Use the above for instanceof immediately followed by ifeq or ifne. * ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.hpp (SharkTopLevelBlock::do_if_helper): New method. (SharkTopLevelBlock::static_subtype_check): Likewise. (SharkTopLevelBlock::maybe_do_instanceof_if): Likewise. (SharkTopLevelBlock::do_optimized_instance_check): Removed. * ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.cpp (SharkTopLevelBlock::do_if_helper): New method. (SharkTopLevelBlock::do_if): Use the above. (SharkTopLevelBlock::static_subtype_check): New method. (SharkTopLevelBlock::do_instance_check): Use the above. (SharkTopLevelBlock::maybe_do_instanceof_if): New method. (SharkTopLevelBlock::do_optimized_instance_check): Removed. (SharkTopLevelBlock::do_full_instance_check): Removed now-unnecessary state copy and merge. * ports/hotspot/src/share/vm/shark/sharkState.hpp (SharkState::replace_all): New method. * ports/hotspot/src/share/vm/shark/sharkState.cpp (SharkState::replace_all): Likewise.
author Andrew John Hughes <ahughes@redhat.com>
date Wed, 10 Jun 2009 17:17:50 +0100
parents 8783e4b809e0
children 12b58111793f
files ChangeLog ports/hotspot/src/share/vm/shark/sharkBlock.cpp ports/hotspot/src/share/vm/shark/sharkBlock.hpp ports/hotspot/src/share/vm/shark/sharkState.cpp ports/hotspot/src/share/vm/shark/sharkState.hpp ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.cpp ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.hpp
diffstat 7 files changed, 200 insertions(+), 38 deletions(-) [+]
line wrap: on
line diff
--- a/ChangeLog	Fri Jun 05 14:46:31 2009 +0100
+++ b/ChangeLog	Wed Jun 10 17:17:50 2009 +0100
@@ -1,3 +1,32 @@
+2009-06-06  Gary Benson  <gbenson@redhat.com>
+
+	* ports/hotspot/src/share/vm/shark/sharkBlock.hpp
+	(SharkBlock::maybe_do_instanceof_if): New method.
+	* ports/hotspot/src/share/vm/shark/sharkBlock.cpp
+	(SharkBlock::maybe_do_instanceof_if): New method.
+	(SharkBlock::parse_bytecode): Use the above for
+	instanceof immediately followed by ifeq or ifne.
+
+	* ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.hpp
+	(SharkTopLevelBlock::do_if_helper): New method.
+	(SharkTopLevelBlock::static_subtype_check): Likewise.
+	(SharkTopLevelBlock::maybe_do_instanceof_if): Likewise.
+	(SharkTopLevelBlock::do_optimized_instance_check): Removed.
+	* ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.cpp
+	(SharkTopLevelBlock::do_if_helper): New method.
+	(SharkTopLevelBlock::do_if): Use the above.
+	(SharkTopLevelBlock::static_subtype_check): New method.
+	(SharkTopLevelBlock::do_instance_check): Use the above.
+	(SharkTopLevelBlock::maybe_do_instanceof_if): New method.
+	(SharkTopLevelBlock::do_optimized_instance_check): Removed.
+	(SharkTopLevelBlock::do_full_instance_check): Removed
+	now-unnecessary state copy and merge.
+
+	* ports/hotspot/src/share/vm/shark/sharkState.hpp
+	(SharkState::replace_all): New method.
+	* ports/hotspot/src/share/vm/shark/sharkState.cpp
+	(SharkState::replace_all): Likewise.
+
 2009-06-06  Gary Benson  <gbenson@redhat.com>
 
 	* ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.hpp
--- a/ports/hotspot/src/share/vm/shark/sharkBlock.cpp	Fri Jun 05 14:46:31 2009 +0100
+++ b/ports/hotspot/src/share/vm/shark/sharkBlock.cpp	Wed Jun 10 17:17:50 2009 +0100
@@ -825,8 +825,38 @@
       do_call();
       break;
 
+    case Bytecodes::_instanceof:
+      // This is a very common construct:
+      //
+      //  if (object instanceof Klass) {
+      //    something = (Klass) object;
+      //    ...
+      //  }
+      //
+      // which gets compiled to something like this:
+      //
+      //  28: aload 9
+      //  30: instanceof <Class Klass>
+      //  33: ifeq 52
+      //  36: aload 9
+      //  38: checkcast <Class Klass>
+      //
+      // Handling both bytecodes at once allows us
+      // to eliminate the checkcast.
+      if (iter()->next_bci() < limit &&
+          (iter()->next_bc() == Bytecodes::_ifeq ||
+           iter()->next_bc() == Bytecodes::_ifne) &&
+          (!UseLoopSafepoints ||
+           iter()->next_get_dest() > iter()->next_bci())) {
+        if (maybe_do_instanceof_if()) {
+          iter()->next();
+          if (SharkTraceBytecodes)
+            tty->print_cr("%4d: %s", bci(), Bytecodes::name(bc()));
+          break;
+        }
+      }
+      // fall through
     case Bytecodes::_checkcast:
-    case Bytecodes::_instanceof:
       do_instance_check();
       break;
 
@@ -1225,6 +1255,11 @@
   ShouldNotCallThis();
 }
 
+bool SharkBlock::maybe_do_instanceof_if()
+{
+  ShouldNotCallThis();
+}
+
 void SharkBlock::do_new()
 {
   ShouldNotCallThis();
--- a/ports/hotspot/src/share/vm/shark/sharkBlock.hpp	Fri Jun 05 14:46:31 2009 +0100
+++ b/ports/hotspot/src/share/vm/shark/sharkBlock.hpp	Wed Jun 10 17:17:50 2009 +0100
@@ -326,6 +326,7 @@
   // checkcast and instanceof
  protected:
   virtual void do_instance_check();
+  virtual bool maybe_do_instanceof_if();
 
   // new and *newarray
  protected:
--- a/ports/hotspot/src/share/vm/shark/sharkState.cpp	Fri Jun 05 14:46:31 2009 +0100
+++ b/ports/hotspot/src/share/vm/shark/sharkState.cpp	Wed Jun 10 17:17:50 2009 +0100
@@ -225,6 +225,21 @@
   set_has_safepointed(this->has_safepointed() && other->has_safepointed());
 }
 
+void SharkState::replace_all(SharkValue* old_value, SharkValue* new_value)
+{
+  // Local variables
+  for (int i = 0; i < max_locals(); i++) {
+    if (local(i) == old_value)
+      set_local(i, new_value);
+  }
+
+  // Expression stack
+  for (int i = 0; i < stack_depth(); i++) {
+    if (stack(i) == old_value)
+      set_stack(i, new_value);
+  }
+}
+
 void SharkState::decache_for_Java_call(ciMethod* callee)
 {
   assert(function() && method(), "you cannot decache here");
--- a/ports/hotspot/src/share/vm/shark/sharkState.hpp	Fri Jun 05 14:46:31 2009 +0100
+++ b/ports/hotspot/src/share/vm/shark/sharkState.hpp	Wed Jun 10 17:17:50 2009 +0100
@@ -187,6 +187,10 @@
              llvm::BasicBlock* other_block,
              llvm::BasicBlock* this_block);
 
+  // Value replacement
+ public:
+  void replace_all(SharkValue* old_value, SharkValue* new_value);
+
   // Cache and decache
  public:
   void decache_for_Java_call(ciMethod* callee);
--- a/ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.cpp	Fri Jun 05 14:46:31 2009 +0100
+++ b/ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.cpp	Wed Jun 10 17:17:50 2009 +0100
@@ -772,8 +772,11 @@
 }
 
 // All propagation of state from one block to the next (via
-// dest->add_incoming) is handled by the next three methods
-// (do_branch, do_if and do_switch) and by handle_exception.
+// dest->add_incoming) is handled by these methods:
+//   do_branch
+//   do_if_helper
+//   do_switch
+//   handle_exception
 
 void SharkTopLevelBlock::do_branch(int successor_index)
 {
@@ -795,16 +798,24 @@
     llvm_a = a->jint_value();
     llvm_b = b->jint_value();
   }
+  do_if_helper(p, llvm_b, llvm_a, current_state(), current_state());
+}
 
+void SharkTopLevelBlock::do_if_helper(ICmpInst::Predicate p,
+                                      Value*              b,
+                                      Value*              a,
+                                      SharkState*         if_taken_state,
+                                      SharkState*         not_taken_state)
+{
   SharkTopLevelBlock *if_taken  = successor(ciTypeFlow::IF_TAKEN);
   SharkTopLevelBlock *not_taken = successor(ciTypeFlow::IF_NOT_TAKEN);
 
   builder()->CreateCondBr(
-    builder()->CreateICmp(p, llvm_a, llvm_b),
+    builder()->CreateICmp(p, a, b),
     if_taken->entry_block(), not_taken->entry_block());
 
-  if_taken->add_incoming(current_state());
-  not_taken->add_incoming(current_state());
+  if_taken->add_incoming(if_taken_state);
+  not_taken->add_incoming(not_taken_state);
 }
 
 void SharkTopLevelBlock::do_switch()
@@ -1164,40 +1175,48 @@
   current_state()->set_has_safepointed(true);
 }
 
+bool SharkTopLevelBlock::static_subtype_check(ciKlass* check_klass,
+                                              ciKlass* object_klass)
+{
+  // If the class we're checking against is java.lang.Object
+  // then this is a no brainer.  Apparently this can happen
+  // in reflective code...
+  if (check_klass == function()->env()->Object_klass())
+    return true;
+
+  // Perform a subtype check.  NB in opto's code for this
+  // (GraphKit::static_subtype_check) it says that static
+  // interface types cannot be trusted, and if opto can't
+  // trust them then I assume we can't either.
+  if (!object_klass->is_interface()) {
+    if (object_klass == check_klass)
+      return true;
+
+    if (object_klass->is_loaded() && check_klass->is_loaded()) {
+      if (object_klass->is_subtype_of(check_klass))
+        return true;
+    }
+  }
+
+  return false;
+}
+  
 void SharkTopLevelBlock::do_instance_check()
 {
   // Get the class we're checking against
   bool will_link;
   ciKlass *check_klass = iter()->get_klass(will_link);
 
-  // If the class we're checking against is java.lang.Object
-  // then this is a no brainer.  Apparently this can happen
-  // in reflective code...
-  if (check_klass == function()->env()->Object_klass()) {
-    do_optimized_instance_check();
-    return;
-  }
-  
   // Get the class of the object we're checking
   ciKlass *object_klass = xstack(0)->type()->as_klass();
 
-  // If the classes are defined enough now then we
-  // don't need a runtime check.  NB opto's code for
-  // this (GraphKit::static_subtype_check) says we
-  // cannot trust static interface types yet, hence
-  // the extra check
-  if (!object_klass->is_interface()) {
-    if (object_klass == check_klass) {
-      do_optimized_instance_check();
-      return;
+  // Can we optimize this check away?
+  if (static_subtype_check(check_klass, object_klass)) {
+    if (bc() == Bytecodes::_instanceof) {
+      pop();
+      push(SharkValue::jint_constant(1));
     }
-
-    if (object_klass->is_loaded() && check_klass->is_loaded()) {
-      if (object_klass->is_subtype_of(check_klass)) {
-        do_optimized_instance_check();
-        return;
-      }
-    }
+    return;
   }
 
   // Need to check this one at runtime
@@ -1207,14 +1226,69 @@
     do_trapping_instance_check(check_klass);
 }
                 
-void SharkTopLevelBlock::do_optimized_instance_check()
+bool SharkTopLevelBlock::maybe_do_instanceof_if()
 {
-  if (bc() == Bytecodes::_instanceof) {
-    pop();
-    push(SharkValue::jint_constant(1));
+  // Get the class we're checking against
+  bool will_link;
+  ciKlass *check_klass = iter()->get_klass(will_link);
+
+  // If the class is unloaded then the instanceof
+  // cannot possibly succeed.
+  if (!will_link)
+    return false;
+
+  // Keep a copy of the object we're checking
+  SharkValue *old_object = xstack(0);
+  
+  // Get the class of the object we're checking
+  ciKlass *object_klass = old_object->type()->as_klass();
+
+  // If the instanceof can be optimized away at compile time
+  // then any subsequent checkcasts will be too so we handle
+  // it normally.
+  if (static_subtype_check(check_klass, object_klass))
+    return false;
+
+  // Perform the instance check
+  do_full_instance_check(check_klass);
+  Value *result = pop()->jint_value();
+
+  // Create the casted object
+  SharkValue *new_object = SharkValue::create_generic(
+    check_klass, old_object->jobject_value(), old_object->zero_checked());
+
+  // Create two copies of the current state, one with the
+  // original object and one with all instances of the
+  // original object replaced with the new, casted object.
+  SharkState *new_state = current_state();
+  SharkState *old_state = new_state->copy();
+  new_state->replace_all(old_object, new_object);
+
+  // Perform the check-and-branch
+  switch (iter()->next_bc()) {
+  case Bytecodes::_ifeq:
+    // branch if not an instance
+    do_if_helper(
+      ICmpInst::ICMP_EQ,
+      LLVMValue::jint_constant(0), result,
+      old_state, new_state);
+    break;
+
+  case Bytecodes::_ifne:
+    // branch if an instance
+    do_if_helper(
+      ICmpInst::ICMP_NE,
+      LLVMValue::jint_constant(0), result,
+      new_state, old_state);
+    break;
+
+  default:
+    ShouldNotReachHere();
   }
+
+  return true;
 }
-    
+
 void SharkTopLevelBlock::do_full_instance_check(ciKlass* klass)
 { 
   BasicBlock *not_null      = function()->CreateBlock("not_null");
@@ -1238,7 +1312,6 @@
     builder()->CreateICmpEQ(object, LLVMValue::null()),
     merge2, not_null);
   BasicBlock *null_block = builder()->GetInsertBlock();
-  SharkState *null_state = current_state()->copy();
 
   // Get the class we're checking against
   builder()->SetInsertPoint(not_null);
@@ -1282,7 +1355,6 @@
   
   // Second merge
   builder()->SetInsertPoint(merge2);
-  current_state()->merge(null_state, null_block, nonnull_block);
   PHINode *result = builder()->CreatePHI(
     SharkType::jint_type(), "result");
   result->addIncoming(LLVMValue::jint_constant(IC_IS_NULL), null_block);
--- a/ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.hpp	Fri Jun 05 14:46:31 2009 +0100
+++ b/ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.hpp	Wed Jun 10 17:17:50 2009 +0100
@@ -358,6 +358,11 @@
 
   // if*
  private:
+  void do_if_helper(llvm::ICmpInst::Predicate p,
+                    llvm::Value*              b,
+                    llvm::Value*              a,
+                    SharkState*               if_taken_state,
+                    SharkState*               not_taken_state);
   void do_if(llvm::ICmpInst::Predicate p, SharkValue* b, SharkValue* a);
 
   // tableswitch and lookupswitch
@@ -386,11 +391,12 @@
 
   // checkcast and instanceof
  private:
-  void do_optimized_instance_check();
+  bool static_subtype_check(ciKlass* check_klass, ciKlass* object_klass);
   void do_full_instance_check(ciKlass* klass);
   void do_trapping_instance_check(ciKlass* klass);
 
   void do_instance_check();
+  bool maybe_do_instanceof_if();
 
   // new and *newarray
  private: