review (XS) for 6889869: assert(!Interpreter::bytecode_should_reexecute(code), "should not reexecute")

[Tom Rodriguez]

I'd considered adding an is_same_info method to PcDesc and that's a  
good reason to do it.  Actually looking at it some more there's  
another latent issue because it doesn't check _obj_decode_offset  
either. I'll change it do this.   openjdk.java.net is still borked so  
I'll update the webrev when it's back but it's basically this:

diff -r 03b336640699 src/share/vm/code/pcDesc.hpp
--- a/src/share/vm/code/pcDesc.hpp      Wed Oct 07 15:38:37 2009 -0700
+++ b/src/share/vm/code/pcDesc.hpp      Mon Oct 12 13:11:54 2009 -0700
@@ -39,6 +39,7 @@ class PcDesc VALUE_OBJ_CLASS_SPEC {
      struct {
        unsigned int reexecute: 1;
      } bits;
+    operator ==(const PcDescFlags& other) { return word ==  
other.word; }
    } _flags;

   public:
@@ -64,6 +65,13 @@ class PcDesc VALUE_OBJ_CLASS_SPEC {
    bool     should_reexecute()              const { return  
_flags.bits.reexecute; }
    void set_should_reexecute(bool z)               
{ _flags.bits.reexecute = z;    }

+  // Does pd refer to the same information as pd?
+  bool is_same_info(const PcDesc* pd) {
+    return _scope_decode_offset == pd->_scope_decode_offset &&
+      _obj_decode_offset == pd->_obj_decode_offset &&
+      _flags == pd->_flags;
+  }
+
    // Returns the real pc
    address real_pc(const nmethod* code) const;

diff -r 03b336640699 src/share/vm/code/debugInfoRec.cpp
--- a/src/share/vm/code/debugInfoRec.cpp        Wed Oct 07 15:38:37  
2009 -0700
+++ b/src/share/vm/code/debugInfoRec.cpp        Mon Oct 12 13:07:49  
2009 -0700
@@ -356,8 +356,7 @@ void DebugInformationRecorder::end_scope
      // search forward until it finds last.
      // In addition, it does not matter if the last PcDesc
      // is for a safepoint or not.
-    if (_prev_safepoint_pc < prev->pc_offset() &&
-        prev->scope_decode_offset() == last->scope_decode_offset()) {
+    if (_prev_safepoint_pc < prev->pc_offset() && prev- 
 >is_same_info(last)) {
        assert(prev == last-1, "sane");
        prev->set_pc_offset(pc_offset);
        _pcs_length -= 1;

tom

On Oct 12, 2009, at 12:41 AM, Christian Thalinger wrote:

> On Thu, 2009-10-08 at 16:40 -0700, Tom Rodriguez wrote:
>> http://cr.openjdk.java.net/~never/6889869/
>
> I guess I have to do the same change for the is_method_handle_invoke
> flag.  Maybe we should compare the whole _flags.word instead of every
> single bit in there?
>
> -- Christian
>