EVALUATION
The Fujitsu evaluation sounds correct. An "if" bytecode cannot currently be
rewritten to a wide version, since none exists. I am working on a relocator
change that will change the code from:
#1 if <cond> <wide delta>
#2 <else code>
to:
#1 if <cont> #2
#2 goto #4
#3 goto_w <wide delta>
#4 <else code>
I don't really have anything to test it on. Can Fujitsu send a class file
sample? Or could I send them my changes and have them test them out? Thanks!
###@###.### 2003-10-24
I marked this incomplete to get it off the hot list until Fujitsu can try out
the fix I put in "suggested fix".
###@###.### 2003-11-18
Correction, the new bytecode sequence is:
#1 if <cond> #3
#2 goto #4
#3 goto_w <wide delta>
#4 <else code>
I have a test case to test it on now and the code is debugged and working.
###@###.### 2003-12-09
Putback comments:
Several users in the field have been getting the ShouldNotReachHere() from
relocator.cpp because support for the branch destination of "if" bytecodes
could not be widened past a short.
In this fix, I have rewritten the bytecode stream for widening "if" from:
#1 if <cond> <wide delta>
#2 <else code>
to:
#1 if <cont> #3
#2 goto #4
#3 goto_w <wide delta>
#4 <else code>
This has been debugged and tested for a testcase that Mingyao wrote that meets
all the conditions for this rewriting (appropriately placed jsr's and an
if branch needing widening to 32 bits). I was hoping Fujitsu would test
this out but I never heard from them.
Risk assessment: This is a corner case, which previously asserted.
There have been lots of customer complaints about this assertion so this
fix should be backported.
Also, fixed was the bitfield length calculation for representing bits for each
bci in basic blocks. If there was >1 rewriting towards the end of the
method, the bit vector would overflow and assert.
Also fixed: When pushing jump relocations on the _changes worklist, the delta
offset to use was wrong if the jump relocation was already on the worklist. The
second request was ignored if it had already been relocated. This was wrong
because if the bci delta needed further adjustment, that adjustment was never
made. This was seen with another of Mingyao's test cases where two branches
needed adjustment and the adjustment of the first (+2) required that the second
delta be further adjusted. The fix was to scan the worklist and modify the
delta rather than pushing a duplicate request on the list with a delta that
was derived from the original non-adjusted code stream.
See suggested fix for webrev.
###@###.### 2003-12-19
|
SUGGESTED FIX
*** /src/share/vm/runtime/relocator.cpp- Tue Nov 18 14:23:57 2003
--- relocator.cpp Tue Nov 18 14:19:48 2003
*** 1,7 ****
#ifdef USE_PRAGMA_IDENT_SRC
! #pragma ident "@(#)relocator.cpp 1.27 03/01/23 12:24:46 JVM"
#endif
/*
* Copyright 2003 Sun Microsystems, Inc. All rights reserved.
* SUN PROPRIETARY/CONFIDENTIAL. Use is subject to license terms.
*/
--- 1,7 ----
#ifdef USE_PRAGMA_IDENT_SRC
! #pragma ident "@(#)relocator.cpp 1.28 03/11/18 14:19:48 JVM"
#endif
/*
* Copyright 2003 Sun Microsystems, Inc. All rights reserved.
* SUN PROPRIETARY/CONFIDENTIAL. Use is subject to license terms.
*/
*** 469,491 ****
// handle jump_widen instruction. Called be ChangeJumpWiden class
bool Relocator::handle_jump_widen(int bci, int delta) {
int ilen = rc_instr_len(bci);
if (ilen != 3) {
// Request already handled
assert(code_at(bci) == Bytecodes::_goto_w || code_at(bci) == Bytecodes::_jsr_w, "sanity check");
return true;
}
-
assert(ilen == 3, "check length");
if (!relocate_code(bci, 3, 2)) return false;
- Bytecodes::Code bc = code_at(bci);
- switch (bc) {
- case Bytecodes::_goto: code_at_put(bci, Bytecodes::_goto_w); break;
- case Bytecodes::_jsr: code_at_put(bci, Bytecodes::_jsr_w); break;
default: ShouldNotReachHere();
}
// If it's a forward jump, add 2 for the widening.
if (delta > 0) delta += 2;
--- 469,540 ----
// handle jump_widen instruction. Called be ChangeJumpWiden class
bool Relocator::handle_jump_widen(int bci, int delta) {
int ilen = rc_instr_len(bci);
+ Bytecodes::Code bc = code_at(bci);
+ switch (bc) {
+ case Bytecodes::_ifeq:
+ case Bytecodes::_ifne:
+ case Bytecodes::_iflt:
+ case Bytecodes::_ifge:
+ case Bytecodes::_ifgt:
+ case Bytecodes::_ifle:
+ case Bytecodes::_if_icmpeq:
+ case Bytecodes::_if_icmpne:
+ case Bytecodes::_if_icmplt:
+ case Bytecodes::_if_icmpge:
+ case Bytecodes::_if_icmpgt:
+ case Bytecodes::_if_icmple:
+ case Bytecodes::_if_acmpeq: {
+ // If 'if' points to the next bytecode, it's already handled.
+ if (short_at(bci+1) == ilen) {
+ return true;
+ }
+ assert(ilen == 3, "check length");
+
+ // Convert to 0 if <cond> goto 6
+ // 3 _goto 10
+ // 6 _goto_w <wide delta offset>
+ // 11 <else code>
+ const int goto_bci = Bytecodes::length_for(Bytecodes::_goto);
+ const int goto_w_bci = Bytecodes::length_for(Bytecodes::_goto_w);
+ const int add_bci = goto_bci + goto_w_bci;
+
+ if (!relocate_code(bci, 3, /*delta*/add_bci)) return false;
+
+ short_at_put(bci + 1, ilen);
+
+ int cbci = bci + ilen;
+ // goto around
+ code_at_put(cbci, Bytecodes::_goto);
+ short_at_put(cbci + 1, add_bci);
+ // goto_w <wide delta>
+ cbci = cbci + goto_bci;
+ code_at_put(cbci, Bytecodes::_goto_w);
+ if (delta > 0) delta += 2;
+ int_at_put(cbci + 1, delta);
+ break;
+
+ }
+ case Bytecodes::_if_acmpne:
+ case Bytecodes::_goto:
+ case Bytecodes::_jsr:
if (ilen != 3) {
// Request already handled
assert(code_at(bci) == Bytecodes::_goto_w || code_at(bci) == Bytecodes::_jsr_w, "sanity check");
return true;
}
assert(ilen == 3, "check length");
+
if (!relocate_code(bci, 3, 2)) return false;
+ if (bc == Bytecodes::_goto)
+ code_at_put(bci, Bytecodes::_goto_w);
+ else
+ code_at_put(bci, Bytecodes::_jsr_w);
+ break;
default: ShouldNotReachHere();
}
// If it's a forward jump, add 2 for the widening.
if (delta > 0) delta += 2;
###@###.### 2003-12-19
See webrev:
http://jruntime.east/~coleenp/net/philli.east/files/coleenp/webrev/relocation/index.html
Or:
http://jruntime.east/~coleenp/webrev/4879051/
for potential backport.
|
Java