harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tianwei <tianwei.sh...@gmail.com>
Subject Re: MIPS Progress (WAS: The progress of MIPS patch work)
Date Tue, 05 Jan 2010 12:19:10 GMT
Hi,Charles,
   Thanks. Since your patch does not include the makefile modification,
Maybe I can  send another patch which includes the makefile fixes based on
your new patch. I think the most important requirements for the patch is
that it should not break any existing things.
   BTW, can anyone point me to the right document for how to be involved in
harmony development, i.e, checkin rules, licence issues, etc?

Thanks.

Tianwei

On Tue, Jan 5, 2010 at 6:40 AM, Charles Hardin <ckhardin@gmail.com> wrote:

> 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
>



-- 
Sheng, Tianwei
Inst. of High Performance Computing
Dept. of Computer Sci. & Tech.
Tsinghua Univ.

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message