Hi, I forgot to say the CLA has been completed and sent. I guess you should get it soon. Also I wanted to know about the headers and copyrights for mips files. Can I add a line like "Copyright (c) Sigma Designs." or maybe a mention like this for files with Sun copyright: "The original source code has been modified significantly modified by Sigma Designs."
Thanks, Alexandre On Jan 25, 4:27 pm, Alexandre Rames <[email protected]> wrote: > Hi Søren, > > I uploaded a new version of the code. > > About code generation, I have actually already implemented much of the > CodeGenerator. I used the mainly the ARM code and also a little of the > x86 code to help me implement it for MIPS. However I did not look at > the FullCodeGen yet. > However at the end I could not manage to have everything work > properly. Committing everything bits by bits is good way to check > everyting again and have external comments on my code. > > I was trying to have the sample shell work. I passed the InstallNatives > () (with default natives files) in bootstrapper.cc line 1550, but > failed to setup custom C functions in ConfigureGlobalObjects(). > > I think the next step, outside from correcting the current code and > style errors, should be to add more assembler tests, and maybe > generate some simple code for integers operations. > > I'll start with more assembler tests, and wait for your opinion on > what to implement next. > > Regards, > > Alexandre > > On Jan 25, 6:08 am, Søren Gjesse <[email protected]> wrote: > > > > > Alexandre, > > > Thank you very much for the comments > > tohttp://codereview.chromium.org/549079. I think you should upload the > > changes > > to laterhttp://codereview.chromium.org/543161andcontinue the code review > > there. > > > Returning to code generation, the suggestion to go for implementing the > > FullCodeGen might not be the best way to go after all. As I indicated it is > > a moving target, and you would probably run into changes that conflicts with > > what you are doing. Taking the CodeGen instead, without using the virtual > > frame and register allocation could be an easier way forward. CodeGen will > > not change much, and have been a solid backend from the initial V8 version. > > Without the virtual frame and register allocation you should have the stack > > manipulation explicitly in the generated code and handle branching using > > plain labels and not JumpTarget. The ARM port is actually a bit like this > > already. It has hooks into the virtual frame, but does not use it that much, > > and currently there is no register allocation for ARM. > > > Regards, > > Søren > > > On Fri, Jan 22, 2010 at 21:23, Søren Gjesse <[email protected]> wrote: > > > Alexandre, > > > > Good that gcl is now working for you again. I have reviewed the change a > > > bit further, but still mostly general build related comments and style > > > nits. > > > After we have these basic things sorted we can move on to the real > > > substance > > > of the change. > > > > Also I want to give you a little description of the review process. First > > > of all on your first uploaded change ( > > >http://codereview.chromium.org/549079) it would be nice if you could > > > respond to the individual comments to indicate how you have handled it. On > > > your new change I can see that you have addressed some comments and not > > > others. The normal workflow is to address the comments, and after that > > > update the change so that the reviewers can se the changes. This way there > > > can be several iterations on the same change. You can see my latest change > > > athttp://codereview.chromium.org/545151. It has two patch sets, the first > > > being my initial change together with review comments and my responses. > > > The > > > second patch set is after my changes addressing the comments and in there > > > one can see the difference between the patch sets. > > > > To add a new patch is just a matter of running 'gcl upload ...' again. You > > > should note that this works best if the same svn base revision is used. If > > > 'svn up' is run and a new patch set uploaded it can get a bit confusing. I > > > think we should iterate a bit on your new change, but at some point you > > > should move from r3220 to HEAD, and then start a new change. The change > > > needs to fit to HEAD to be committed. > > > > We have also talked a bit about what would be the easiest way for you to > > > start generating code from the AST. The situation is the following: > > > Currently there is two code generators the CodeGen (in files > > > codegen-xxx.*) > > > and the FastCodeGen (in files fast-codegen-xxx.* - which was renamed to > > > FullCodeGen in r3660 and r3662). CodeGen is the original code generator > > > which have evolved from V8 version 1.0 to its current state with the > > > virtual > > > frame handling and lots of optimizations. The FastCodeGen was introduced > > > mainly to speedup loading of web pages. The fast in its name means > > > generate > > > code fast and don't do so many optimizations. The reason for this was that > > > all the optimizations added to CodeGen made the code itself run faster but > > > the code generation became slower. Currently FastCodeGen is used for most > > > top-level code. Some JavaScript constructs are not supported by > > > FastCodeGen > > > and code generation is then handled by CodeGen. However the rename to > > > FullCodeGen is to indicate that is will support all JavaScript constructs, > > > and we are quite close. The current CodeGen is then supposed to be phased > > > out and replaced by a new optimizing compiler which might not support all > > > JavaScript constructs but generate highly optimized code for the one it > > > does. FullCodeGen will be used for the rest. > > > > As you can see code generation in V8 is a moving target, and plans might > > > still change. Even so our suggestion is that you start with looking at > > > implementing FullCodeGen, as it is simpler than CodeGen. That will > > > probably > > > be the fastest path to generating code for the empty function, executing > > > it > > > and getting undefined back. From there you can add constructs until all of > > > JavaScript is supported and you can build V8 with the builtin JavaScript > > > and > > > pass all the tests. As work is still in progress on the FullCodeGen you > > > might encounter some setbacks, but hopefully not unreasonable ones. > > > > Regards, > > > Søren > > > > On Fri, Jan 22, 2010 at 00:56, Alexandre Rames > > > <[email protected]>wrote: > > > >> It was indeed a conflict between mercurial and svn. > > >> I uploaded the changes. I'm still working on the simulator, but it can > > >> already be used. > > > >> Alexandre > > > >> On Jan 21, 2:15 pm, Alexandre Rames <[email protected]> wrote: > > >> > Oh! > > >> > It might be because I also use a mercurial repository in the same > > >> > folder to keep track of my work. I did not think of this. > > >> > I'll check that. > > > >> > Thanks, > > > >> > Alexandre > > > >> > On Jan 21, 1:54 pm, Søren Gjesse <[email protected]> wrote: > > > >> > > That looks quite strange. It looks as if gcl tries to use the 'hg' > > >> > > (Mercurial Distributed SCM) command. If you look at > > > >> > >http://groups.google.com/group/chromium-dev/browse_thread/thread/e775. > > >> .. > > > >> > > I would have expected to see 'svn' in place of 'hg' in the error > > >> message. > > > >> > > There is a few mentions of Mercurial on the mailing list > > > >> > >http://groups.google.com/group/chromium-dev/browse_thread/thread/affc. > > >> .. > > > >> > > I will suggest that you take a look at GuessVCS/GuessVCSName in > > >> > > depot_tools/upload.py, to figure out what is happening. > > > >> > > Regards, > > >> > > Søren > > > >> > > On Thu, Jan 21, 2010 at 20:25, Alexandre Rames < > > >> [email protected]>wrote: > > > >> > > > I'm sorry but I don't manage to upload my changes... > > >> > > > I created a fresh checkout of revision 3220, added mips files and > > >> > > > modified the other. > > >> > > > gcl opened seems to indicate everything is fine, but when I try to > > >> > > > upload I get this error: > > > >> > > > `--> gcl upload 2ndMIPSCommit > > >> > > > Got error status from ['hg', 'cat', '-r', 'cd8114d1dfc6', > > >> 'test/cctest/ > > >> > > > test-assembler-mips.cc']: > > > >> > > > I tried to remove test-assembler-mips.cc but it did not help. Any > > >> > > > idea? > > > >> > > > gcl opened outputs the following: > > >> > > > `--> gcl opened > > >> > > > --- Changelist 2ndMIPSCommit: > > >> > > > A test/cctest/test-assembler-mips.cc > > >> > > > M src/spaces-inl.h > > >> > > > M src/register-allocator.h > > >> > > > M src/heap.cc > > >> > > > M src/codegen-inl.h > > >> > > > M src/bootstrapper.cc > > >> > > > M src/flag-definitions.h > > >> > > > M src/jump-target.cc > > >> > > > M src/regexp-macro-assembler.cc > > >> > > > M src/codegen.h > > >> > > > M src/runtime.js > > >> > > > M src/virtual-frame.h > > >> > > > M src/objects.h > > >> > > > M src/SConscript > > >> > > > M src/macro-assembler.h > > >> > > > M src/assembler.h > > >> > > > M src/v8.cc > > >> > > > M src/platform-linux.cc > > >> > > > M src/frames-inl.h > > >> > > > M src/objects-inl.h > > >> > > > M src/jump-target.h > > >> > > > M src/code-stubs.h > > >> > > > M src/heap.h > > >> > > > M src/register-allocator-inl.h > > >> > > > A src/mips > > >> > > > A src/mips/codegen-mips-inl.h > > >> > > > A src/mips/ic-mips.cc > > >> > > > A src/mips/assembler-mips-inl.h > > >> > > > A src/mips/jump-target-mips.cc > > >> > > > A src/mips/register-allocator-mips.cc > > >> > > > A src/mips/codegen-mips.cc > > >> > > > A src/mips/regexp-macro-assembler-mips.h > > >> > > > A src/mips/constants-mips.cc > > >> > > > A src/mips/frames-mips.cc > > >> > > > A src/mips/virtual-frame-mips.cc > > >> > > > A src/mips/macro-assembler-mips.h > > >> > > > A src/mips/frames-mips.h > > >> > > > A src/mips/disasm-mips.cc > > >> > > > A src/mips/debug-mips.cc > > >> > > > A src/mips/cpu-mips.cc > > >> > > > A src/mips/builtins-mips.cc > > >> > > > A src/mips/fast-codegen-mips.cc > > >> > > > A src/mips/readme > > >> > > > A src/mips/register-allocator-mips.h > > >> > > > A src/mips/regexp-macro-assembler-mips.cc > > >> > > > A src/mips/codegen-mips.h > > >> > > > A src/mips/macro-assembler-mips.cc > > >> > > > A src/mips/assembler-mips.cc > > >> > > > A src/mips/test-interface-mips.cc > > ... > > read more » -- v8-users mailing list [email protected] http://groups.google.com/group/v8-users
