harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Charles Hardin <ckhar...@gmail.com>
Subject MIPS Progress (WAS: The progress of MIPS patch work)
Date Mon, 04 Jan 2010 22:40:46 GMT
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.sheng@gmail.com> wrote:
> Hi, Charles,
>   I finally made a milestone, and get the interpreter works for
> "HelloWorld", the following is the result:
> stw@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.sheng@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

Mime
View raw message