/hg/icedtea6: Fix PR icedtea/494 (and speed up exception handling)

gbenson at icedtea.classpath.org gbenson at icedtea.classpath.org
Thu May 20 04:41:40 PDT 2010


changeset c8cdcd3a1f01 in /hg/icedtea6
details: http://icedtea.classpath.org/hg/icedtea6?cmd=changeset;node=c8cdcd3a1f01
author: Gary Benson <gbenson at redhat.com>
date: Thu May 20 12:40:31 2010 +0100

	Fix PR icedtea/494 (and speed up exception handling)

	This commit fixes PR icedtea/494, a bug where some exception
	handlers were not installed. Shark relied on exception handler
	tables supplied by the TypeFlow pass, but it turns out that TypeFlow
	does not record all exceptions in its tables. Shark now builds its
	own tables.

	While implementing this I realised that the VM call carried out to
	figure out which exception handler to use is unnecessary in most
	cases. I've left the original code in, but added a fast path which
	bypasses a lot of the junk.


diffstat:

4 files changed, 218 insertions(+), 53 deletions(-)
ChangeLog                                               |   27 +
ports/hotspot/src/share/vm/shark/sharkInvariants.hpp    |    7 
ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.cpp |  208 +++++++++++----
ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.hpp |   29 +-

diffs (382 lines):

diff -r 70a50bdf254f -r c8cdcd3a1f01 ChangeLog
--- a/ChangeLog	Thu May 20 11:56:06 2010 +0100
+++ b/ChangeLog	Thu May 20 12:40:31 2010 +0100
@@ -1,3 +1,30 @@ 2010-05-20  Gary Benson  <gbenson at redhat
+2010-05-20  Gary Benson  <gbenson at redhat.com>
+
+	PR icedtea/494
+	* ports/hotspot/src/share/vm/shark/sharkInvariants.hpp
+	(SharkCompileInvariants::java_lang_Throwable_klass): New method.
+	* ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.hpp
+	(SharkTopLevelBlock::_exc_handlers): New field.
+	(SharkTopLevelBlock::_exceptions): Likewise.
+	(SharkTopLevelBlock::compute_exceptions): New method.
+	(SharkTopLevelBlock::num_exceptions): Rewritten.
+	(SharkTopLevelBlock::exc_handler): New method.
+	(SharkTopLevelBlock::exception): Rewritten.
+	(SharkTopLevelBlock::marshal_exception_fast): New method.
+	(SharkTopLevelBlock::marshal_exception_slow): Likewise.
+	(SharkTopLevelBlock::handler_for_exception): Likewise.
+	(SharkTopLevelBlock::make_trap): Likewise.
+	* ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.cpp
+	(SharkTopLevelBlock::enter): Compute exceptions on entry,
+	and skip over any null handlers.
+	(SharkTopLevelBlock::compute_exceptions): New method.
+	(SharkTopLevelBlock::handle_exception): Rewritten.
+	(SharkTopLevelBlock::marshal_exception_fast): New method.
+	(SharkTopLevelBlock::marshal_exception_slow): Likewise.
+	(SharkTopLevelBlock::handler_for_exception): Likewise.
+	(SharkTopLevelBlock::make_trap): Likewise.
+	(SharkTopLevelBlock::can_reach_helper): Skip over null handlers.
+
 2010-05-20  Gary Benson  <gbenson at redhat.com>
 
 	* ports/hotspot/src/cpu/zero/vm/shark_globals_zero.hpp
diff -r 70a50bdf254f -r c8cdcd3a1f01 ports/hotspot/src/share/vm/shark/sharkInvariants.hpp
--- a/ports/hotspot/src/share/vm/shark/sharkInvariants.hpp	Thu May 20 11:56:06 2010 +0100
+++ b/ports/hotspot/src/share/vm/shark/sharkInvariants.hpp	Thu May 20 12:40:31 2010 +0100
@@ -1,6 +1,6 @@
 /*
  * Copyright 1999-2007 Sun Microsystems, Inc.  All Rights Reserved.
- * Copyright 2008, 2009 Red Hat, Inc.
+ * Copyright 2008, 2009, 2010 Red Hat, Inc.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -93,10 +93,13 @@ class SharkCompileInvariants : public Re
     return builder()->code_buffer();
   }
 
-  // That well-known class...
+  // Commonly used classes
  protected:
   ciInstanceKlass* java_lang_Object_klass() const {
     return env()->Object_klass();
+  }
+  ciInstanceKlass* java_lang_Throwable_klass() const {
+    return env()->Throwable_klass();
   }
 };
 
diff -r 70a50bdf254f -r c8cdcd3a1f01 ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.cpp
--- a/ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.cpp	Thu May 20 11:56:06 2010 +0100
+++ b/ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.cpp	Thu May 20 12:40:31 2010 +0100
@@ -221,8 +221,11 @@ void SharkTopLevelBlock::enter(SharkTopL
         successor(i)->enter(this, false);
       }
     }
+    compute_exceptions();
     for (int i = 0; i < num_exceptions(); i++) {
-      exception(i)->enter(this, true);
+      SharkTopLevelBlock *handler = exception(i);
+      if (handler)
+        handler->enter(this, true);
     }
   }
 }
@@ -438,67 +441,93 @@ void SharkTopLevelBlock::check_pending_e
   builder()->SetInsertPoint(no_exception);
 }
 
+void SharkTopLevelBlock::compute_exceptions() {
+  ciExceptionHandlerStream str(target(), start());
+
+  int exc_count = str.count();
+  _exc_handlers = new GrowableArray<ciExceptionHandler*>(exc_count);
+  _exceptions   = new GrowableArray<SharkTopLevelBlock*>(exc_count);
+
+  int index = 0;
+  for (; !str.is_done(); str.next()) {
+    ciExceptionHandler *handler = str.handler();
+    if (handler->handler_bci() == -1)
+      break;
+    _exc_handlers->append(handler);
+
+    // Try and get this exception's handler from typeflow.  We should
+    // do it this way always, really, except that typeflow sometimes
+    // doesn't record exceptions, even loaded ones, and sometimes it
+    // returns them with a different handler bci.  Why???
+    SharkTopLevelBlock *block = NULL;
+    ciInstanceKlass* klass;
+    if (handler->is_catch_all()) {
+      klass = java_lang_Throwable_klass();
+    }
+    else {
+      klass = handler->catch_klass();
+    }
+    for (int i = 0; i < ciblock()->exceptions()->length(); i++) {
+      if (klass == ciblock()->exc_klasses()->at(i)) {
+        block = function()->block(ciblock()->exceptions()->at(i)->pre_order());
+        if (block->start() == handler->handler_bci())
+          break;
+        else
+          block = NULL;
+      }
+    }
+
+    // If typeflow let us down then try and figure it out ourselves
+    if (block == NULL) {
+      for (int i = 0; i < function()->block_count(); i++) {
+        SharkTopLevelBlock *candidate = function()->block(i);
+        if (candidate->start() == handler->handler_bci()) {
+          if (block != NULL) {
+            NOT_PRODUCT(warning("there may be trouble ahead"));
+            block = NULL;
+            break;
+          }
+          block = candidate;
+        }
+      }
+    }
+    _exceptions->append(block);
+  }
+}
+
 void SharkTopLevelBlock::handle_exception(Value* exception, int action) {
   if (action & EAM_HANDLE && num_exceptions() != 0) {
-    // Clear the stack and push the exception onto it.
-    // We do this now to protect it across the VM call
-    // we may be about to make.
+    // Clear the stack and push the exception onto it
     while (xstack_depth())
       pop();
     push(SharkValue::create_jobject(exception, true));
 
-    int *indexes = NEW_RESOURCE_ARRAY(int, num_exceptions());
-    bool has_catch_all = false;
-
-    ciExceptionHandlerStream eh_iter(target(), bci());
-    for (int i = 0; i < num_exceptions(); i++, eh_iter.next()) {
-      ciExceptionHandler* handler = eh_iter.handler();
-
-      if (handler->is_catch_all()) {
-        assert(i == num_exceptions() - 1, "catch-all should be last");
-        has_catch_all = true;
-      }
-      else {
-        indexes[i] = handler->catch_klass_index();
-      }
-    }
-
+    // Work out how many options we have to check
+    bool has_catch_all = exc_handler(num_exceptions() - 1)->is_catch_all();
     int num_options = num_exceptions();
     if (has_catch_all)
       num_options--;
 
-    // Drop into the runtime if there are non-catch-all options
+    // Marshal any non-catch-all handlers
     if (num_options > 0) {
-      Value *index = call_vm(
-        builder()->find_exception_handler(),
-        builder()->CreateInlineData(
-          indexes,
-          num_options * sizeof(int),
-          PointerType::getUnqual(SharkType::jint_type())),
-        LLVMValue::jint_constant(num_options),
-        EX_CHECK_NO_CATCH);
-
-      // Jump to the exception handler, if found
-      BasicBlock *no_handler = function()->CreateBlock("no_handler");
-      SwitchInst *switchinst = builder()->CreateSwitch(
-        index, no_handler, num_options);
-
+      bool all_loaded = true;
       for (int i = 0; i < num_options; i++) {
-        SharkTopLevelBlock* handler = this->exception(i);
-
-        switchinst->addCase(
-          LLVMValue::jint_constant(i),
-          handler->entry_block());
-
-        handler->add_incoming(current_state());
+        if (!exc_handler(i)->catch_klass()->is_loaded()) {
+          all_loaded = false;
+          break;
+        }
       }
 
-      builder()->SetInsertPoint(no_handler);
+      if (all_loaded)
+        marshal_exception_fast(num_options);
+      else
+        marshal_exception_slow(num_options);
     }
 
-    // No specific handler exists, but maybe there's a catch-all
+    // Install the catch-all handler, if present
     if (has_catch_all) {
       SharkTopLevelBlock* handler = this->exception(num_options);
+      assert(handler != NULL, "catch-all handler cannot be unloaded");
 
       builder()->CreateBr(handler->entry_block());
       handler->add_incoming(current_state());
@@ -508,6 +537,78 @@ void SharkTopLevelBlock::handle_exceptio
 
   // No exception handler was found; unwind and return
   handle_return(T_VOID, exception);
+}
+
+void SharkTopLevelBlock::marshal_exception_fast(int num_options) {
+  Value *exception_klass = builder()->CreateValueOfStructEntry(
+    xstack(0)->jobject_value(),
+    in_ByteSize(oopDesc::klass_offset_in_bytes()),
+    SharkType::oop_type(),
+    "exception_klass");
+
+  for (int i = 0; i < num_options; i++) {
+    Value *check_klass =
+      builder()->CreateInlineOop(exc_handler(i)->catch_klass());
+
+    BasicBlock *not_exact   = function()->CreateBlock("not_exact");
+    BasicBlock *not_subtype = function()->CreateBlock("not_subtype");
+
+    builder()->CreateCondBr(
+      builder()->CreateICmpEQ(check_klass, exception_klass),
+      handler_for_exception(i), not_exact);
+
+    builder()->SetInsertPoint(not_exact);
+    builder()->CreateCondBr(
+      builder()->CreateICmpNE(
+        builder()->CreateCall2(
+          builder()->is_subtype_of(), check_klass, exception_klass),
+        LLVMValue::jbyte_constant(0)),
+      handler_for_exception(i), not_subtype);
+
+    builder()->SetInsertPoint(not_subtype);
+  }
+}
+
+void SharkTopLevelBlock::marshal_exception_slow(int num_options) {
+  int *indexes = NEW_RESOURCE_ARRAY(int, num_options);
+  for (int i = 0; i < num_options; i++)
+    indexes[i] = exc_handler(i)->catch_klass_index();
+
+  Value *index = call_vm(
+    builder()->find_exception_handler(),
+    builder()->CreateInlineData(
+      indexes,
+      num_options * sizeof(int),
+      PointerType::getUnqual(SharkType::jint_type())),
+    LLVMValue::jint_constant(num_options),
+    EX_CHECK_NO_CATCH);
+
+  BasicBlock *no_handler = function()->CreateBlock("no_handler");
+  SwitchInst *switchinst = builder()->CreateSwitch(
+    index, no_handler, num_options);
+
+  for (int i = 0; i < num_options; i++) {
+    switchinst->addCase(
+      LLVMValue::jint_constant(i),
+      handler_for_exception(i));
+  }
+
+  builder()->SetInsertPoint(no_handler);
+}
+
+BasicBlock* SharkTopLevelBlock::handler_for_exception(int index) {
+  SharkTopLevelBlock *successor = this->exception(index);
+  if (successor) {
+    successor->add_incoming(current_state());
+    return successor->entry_block();
+  }
+  else {
+    return make_trap(
+      exc_handler(index)->handler_bci(),
+      Deoptimization::make_trap_request(
+        Deoptimization::Reason_unhandled,
+        Deoptimization::Action_reinterpret));
+  }
 }
 
 void SharkTopLevelBlock::maybe_add_safepoint() {
@@ -579,11 +680,28 @@ bool SharkTopLevelBlock::can_reach_helpe
   }
 
   for (int i = 0; i < num_exceptions(); i++) {
-    if (exception(i)->can_reach_helper(other))
+    SharkTopLevelBlock *handler = exception(i);
+    if (handler && handler->can_reach_helper(other))
       return true;
   }
 
   return false;
+}
+
+BasicBlock* SharkTopLevelBlock::make_trap(int trap_bci, int trap_request) {
+  BasicBlock *trap_block = function()->CreateBlock("trap");
+  BasicBlock *orig_block = builder()->GetInsertBlock();
+  builder()->SetInsertPoint(trap_block);
+
+  int orig_bci = bci();
+  iter()->force_bci(trap_bci);
+
+  do_trap(trap_request);
+
+  builder()->SetInsertPoint(orig_block);
+  iter()->force_bci(orig_bci);
+
+  return trap_block;
 }
 
 void SharkTopLevelBlock::do_trap(int trap_request) {
diff -r 70a50bdf254f -r c8cdcd3a1f01 ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.hpp
--- a/ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.hpp	Thu May 20 11:56:06 2010 +0100
+++ b/ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.hpp	Thu May 20 12:40:31 2010 +0100
@@ -79,12 +79,6 @@ class SharkTopLevelBlock : public SharkB
   bool falls_through() const {
     return ciblock()->control() == ciBlock::fall_through_bci;
   }
-  int num_exceptions() const {
-    return ciblock()->exceptions()->length();
-  }
-  SharkTopLevelBlock* exception(int index) const {
-    return function()->block(ciblock()->exceptions()->at(index)->pre_order());
-  }
   int num_successors() const {
     return ciblock()->successors()->length();
   }
@@ -92,6 +86,25 @@ class SharkTopLevelBlock : public SharkB
     return function()->block(ciblock()->successors()->at(index)->pre_order());
   }
   SharkTopLevelBlock* bci_successor(int bci) const;
+
+  // Exceptions
+ private:
+  GrowableArray<ciExceptionHandler*>* _exc_handlers;
+  GrowableArray<SharkTopLevelBlock*>* _exceptions;
+
+ private:
+  void compute_exceptions();
+
+ private:
+  int num_exceptions() const {
+    return _exc_handlers->length();
+  }
+  ciExceptionHandler* exc_handler(int index) const {
+    return _exc_handlers->at(index);
+  }
+  SharkTopLevelBlock* exception(int index) const {
+    return _exceptions->at(index);
+  }
 
   // Traps
  private:
@@ -250,6 +263,9 @@ class SharkTopLevelBlock : public SharkB
   };
   void check_pending_exception(int action);
   void handle_exception(llvm::Value* exception, int action);
+  void marshal_exception_fast(int num_options);
+  void marshal_exception_slow(int num_options);
+  llvm::BasicBlock* handler_for_exception(int index);
 
   // VM calls
  private:
@@ -335,6 +351,7 @@ class SharkTopLevelBlock : public SharkB
 
   // Traps
  private:
+  llvm::BasicBlock* make_trap(int trap_bci, int trap_request);
   void do_trap(int trap_request);
 
   // Returns



More information about the distro-pkg-dev mailing list