The first part of your MIPS work has now been committed as r3799. Thank you very much, and keep up the good work.
Regards, Søren On Wed, Jan 27, 2010 at 16:50, Søren Gjesse <[email protected]> wrote: > Hi Alexandre, > > It cool, that you have worked you way through all of CodeGen, even though > it is not working yet. With that work done I think you should forget about > FullCodeGen for now and focus on getting the CodeGen working. I suggest that > you disable the native files, and start by being able to compile, call and > return from the following JavaScript function: > > function f(){ return 42; } > > and then perhaps move to something like > > function f(){ return 40 + 2; } > > I will continue to look at your change adding the assembler, disassembler > and simulator together with one or two tests of the assembler. You can take > a look at test-assembler-ia32.cc for some simple assembler tests. > > Regarding the copyright it's not necessary for your copyright to be in the > files for the work to be protected under copyright. We very much try to > avoid having lists of names/companies in the copyright notices. However, as > a contributer you will of cause get your name and email in the AUTHORS file. > If Sigma Designs signs the corporate CLA ( > http://code.google.com/legal/corporate-cla-v1.0.html) we can put "Sigma > Designs Inc." in the AUTHORS file as well. > > By the way do you have a reference to the ABI specification (C/C++ calling > conventions) for the (Linux?) platform running on the hardware you are > currently targeting? > > Regards, > Søren > > On Tue, Jan 26, 2010 at 02:18, Alexandre Rames > <[email protected]>wrote: > >> 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 >> > > -- v8-users mailing list [email protected] http://groups.google.com/group/v8-users
