Alexandre, Thank you for the patch, it has been committed r3982.
When creating a change please add an reviewer, and send a review request mail through http://codereview.chromium.org. You can do that through the UI 'Edit Issue' and 'Publish+Mail Comments ('m')', or using options when uploading with gcl. $ gcl upload xxx -r [email protected] --send_mail Regards, Søren On Sat, Feb 27, 2010 at 15:35, Alexandre Rames <[email protected]>wrote: > Hi, > > I am finally back working. > I just merged the MIPS code with the bleeding edge and uploaded the > code. There is little difference with the previous commit, so there > should not be too much work to review it. > > Regards, > Alexandre > > On Feb 4, 10:36 pm, Alexandre Rames <[email protected]> wrote: > > That's great ! > > > > Thank you Søren for correcting the changes from Mads' review. > > > > Regard, > > Alexandre > > > > 2010/2/4 Søren Gjesse <[email protected]> > > > > > > > > > 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/543161andcontinuethe 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 > > > > ... > > > > 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
