LGTM, with a few comments: 1. For some reason, I don't like the name ddest (although it is used consistently). How about something less abbreviated?
2. I would definitely add testers like is_register, is_effect, and is_stack to the DataDestination and remove most of the uses of the DataDestination::Type enum values. 3. Maybe add a helper function in the DataDestination for the "(ddest->type() == DataDestination::REGISTER) ? ddest->reg() : eax" code? 4. We have to be careful not to overdo the undef/def of __. We should probably have a section where we deal with the AST nodes in the codegen-<arch>.cc files. 5. This seems simple enough that you could do the same thing on ARM in this CL. It should be a win on both platforms. Cheers, Kasper On Mon, Sep 29, 2008 at 3:04 PM, <[EMAIL PROTECTED]> wrote: > Reviewers: Kasper Lund, iposva, > > Description: > Begin to be more clever about generating code. > > * When generating code for a literal (an AST leaf), consider the > location we want the value in: top of stack, a register, or don't > care. > > * When generating code for the subexpression of a return statement, > try to generate code to put the value in eax. If not supported, > fall back to the old strategy (push on stack and then pop). > > Thus we never generate push immediate/pop to eax for this case. We > eliminate 6 push/pop eliminations (ie, we don't generate them in the > first place) when starting the VM, and 11 on the V8 benchmarks. > > Please review this at http://codereview.chromium.org/4326 > > Affected files: > M src/assembler-ia32.cc > M src/ast.h > M src/codegen-ia32.cc > > > --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
