Tianwei, Thanks - this is really good news. We should be able to return to looking at the MIPS work in a few weeks.
I'll review the updates you sent and get a new patch submitted to HARMONY-6379 with the updates http://issues.apache.org/jira/browse/HARMONY-6379 Charles On Sat, Jan 2, 2010 at 7:20 AM, Tianwei <tianwei.sh...@gmail.com> wrote: > Hi, Charles, > I finally made a milestone, and get the interpreter works for > "HelloWorld", the following is the result: > s...@rays-b0f748fa:~/test$ > /home/stw/harmony/harmony-nofetch/working_vm/build/linux_mips32_gcc_debug/deploy/jdk/jre/bin/java > -Xint HelloWorldApp > Hello World! > > Please ignore the previous mail, I made a mistake to comment out the > following line in > interp_native_mips.cpp: > > case JAVA_TYPE_DOUBLE: > case JAVA_TYPE_LONG: > > /* MIPS ABI requires 64 bit values to be > naturally aligned. > * pad here and increase array size if required. > */ > if ((argId & 1) != 0) { > sz++; > arg_words2 = (uword*)ALLOC_FRAME((sz + 2) > * sizeof(uword)); > if (!arg_words2) { > DIE(("no memory for frame")); > } > memcpy(arg_words2, arg_words, argId * > sizeof(uword)); > FREE_FRAME(arg_words); > arg_words = arg_words2; > argId++; > } > Value2 val; > val.i64 = args[pos++].j; > arg_words[argId++] = val.v[0].i; > arg_words[argId++] = val.v[1].i; > break; > > I comment out the padding code, this will give me the following error, for > example: > JNIEXPORT void JNICALL > Java_java_util_zip_Inflater_setInputImpl (JNIEnv * env, jobject recv, > jbyteArray buf, jint off, jint > len, > jlong handle) > { > ............ > } > > it has 6 arguments, in the interpreter, before call this JNI, if comment out > the padding code, the args is: > (gdb) p args[4] > $88 = 512 > (gdb) p args[5] > $85 = 10515904 > (gdb) p args[6] > $86 = 0 > > args[5] and args[6] are for jlong, but the MIPS ABI need to align, so we > need to modify it as: > args[4] =512, args[5]=0 > args[6] = 10515904, args[7]=0 > when I enable the padding code, it works. > > Other issues are mainly about get and set value for JAVA_TYPE_LONG, I > believe the original patch has some problems, my fix looks like: > Value2 val; > val.i64 = args[pos++].j; > arg_words[argId++] = val.v[0].i; > arg_words[argId++] = val.v[1].i; > break; > > > summary: > (1) the interpreter works for "HelloWorld" based on Charles's patch on my > MIPS machines. > (2) I only knew very little about the harmony and JVM, only use gdb and > trace to solve the problem, I will continue to > learn them. > (3) next step, I will continue to test more cases and polish my code. > > BTW, Is there any updates for merging your patch into upstream? Since now > I make the interpreter work, I also can help testing. > > Thanks. > > Tianwei > On Tue, Dec 29, 2009 at 9:13 PM, Tianwei <tianwei.sh...@gmail.com> wrote: > >> Well, after two day's debugging, the root cause is that there is some >> problem with the parameter passing in original patch on my machine, some of >> the code looks like: >> --- interpreter.cpp (revision 162) >> >> +++ interpreter.cpp (working copy) >> >> @@ -1691,10 +1691,10 @@ >> >> case VM_DATA_TYPE_INT64: >> >> case VM_DATA_TYPE_F8: >> >> { >> >> - double *vaddr = (double*) addr; >> >> - *vaddr = frame.stack.getLong(0).d; >> >> - frame.stack.pop(2); >> >> - break; >> >> + //double *vaddr = (double*) addr; >> >> + *(I_64*)addr = frame.stack.getLong(0).i64; >> >> + frame.stack.pop(2); >> >> + break; >> >> } >> =================================================================== >> >> --- interp_native_mips.cpp (revision 162) >> >> +++ interp_native_mips.cpp (working copy) >> case JAVA_TYPE_DOUBLE: >> >> case JAVA_TYPE_LONG: >> >> >> - args[argId+0] = prevFrame.stack.pick(pos-0).u; >> >> - args[argId+1] = prevFrame.stack.pick(pos-1).u; >> >> - argId += 2; >> >> - pos -= 2; >> >> - break; >> >> >> >> + Value2 val; >> >> + val.i64 = prevFrame.stack.getLong(pos-1).i64; >> >> + args[argId+0] = val.v[0].i; >> >> + args[argId+1] = val.v[1].i; >> >> + argId += 2; >> >> + pos -= 2; >> >> + break; >> >> I guess the fix is not fully right(does not handle double), but at least >> the orignal patch do not have a consistent usage for "JAVA_TYPE_LONG". >> >> Tianwei
