[v8-dev] Re: ARM: Merging constants in simulator and assembler header files and other clea... (issue6274009)

2011-01-26 Thread ager
LGTM. I'll take care of the last identation nit when landing. Thanks! http://codereview.chromium.org/6274009/diff/40002/src/arm/assembler-arm.cc File src/arm/assembler-arm.cc (right): http://codereview.chromium.org/6274009/diff/40002/src/arm/assembler-arm.cc#newcode1470 src/arm/assembler-arm.cc

[v8-dev] Re: ARM: Merging constants in simulator and assembler header files and other clea... (issue6274009)

2011-01-25 Thread m . m . capewell
http://codereview.chromium.org/6274009/ -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: ARM: Merging constants in simulator and assembler header files and other clea... (issue6274009)

2011-01-25 Thread alexandre . rames
http://codereview.chromium.org/6274009/diff/21001/src/arm/constants-arm.h File src/arm/constants-arm.h (right): http://codereview.chromium.org/6274009/diff/21001/src/arm/constants-arm.h#newcode364 src/arm/constants-arm.h:364: call_rt_redirected = 0x10, Fixed. I missed this one as I nearly missed

[v8-dev] Re: ARM: Merging constants in simulator and assembler header files and other clea... (issue6274009)

2011-01-25 Thread alexandre . rames
Just saw a last comment on a few words' case. I will fix it. http://codereview.chromium.org/6274009/diff/21001/src/arm/assembler-arm.cc File src/arm/assembler-arm.cc (right): http://codereview.chromium.org/6274009/diff/21001/src/arm/assembler-arm.cc#newcode432 src/arm/assembler-arm.cc:432: retu

[v8-dev] Re: ARM: Merging constants in simulator and assembler header files and other clea... (issue6274009)

2011-01-25 Thread m . m . capewell
http://codereview.chromium.org/6274009/ -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: ARM: Merging constants in simulator and assembler header files and other clea... (issue6274009)

2011-01-24 Thread ager
This looks great! A couple of indentation changes and suggestions below. Once they are addressed I think we are ready to land this. We are very inconsistent in our enumerator names. I have put in a comment on the things that I think we should fix before landing this patch. The rest we can

[v8-dev] Re: ARM: Merging constants in simulator and assembler header files and other clea... (issue6274009)

2011-01-21 Thread alexandre . rames
Thanks for the review! Alexandre http://codereview.chromium.org/6274009/diff/1/src/arm/assembler-arm.h File src/arm/assembler-arm.h (right): http://codereview.chromium.org/6274009/diff/1/src/arm/assembler-arm.h#newcode43 src/arm/assembler-arm.h:43: #include "constants-arm.h" On 2011/01/20 12:4

[v8-dev] Re: ARM: Merging constants in simulator and assembler header files and other clea... (issue6274009)

2011-01-21 Thread m . m . capewell
http://codereview.chromium.org/6274009/ -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: ARM: Merging constants in simulator and assembler header files and other clea... (issue6274009)

2011-01-20 Thread ager
I think we should take this opportunity to get rid of all the assembler:: namespaces in the system and just use v8 and v8::internal. This will be a very nice cleanup. :) http://codereview.chromium.org/6274009/diff/1/src/arm/constants-arm.h File src/arm/constants-arm.h (right): http://coderevie

[v8-dev] Re: ARM: Merging constants in simulator and assembler header files and other clea... (issue6274009)

2011-01-20 Thread ager
First overall comment: no using directives. :) I'll have a look at the actual contents soon. http://codereview.chromium.org/6274009/diff/1/src/arm/assembler-arm.h File src/arm/assembler-arm.h (right): http://codereview.chromium.org/6274009/diff/1/src/arm/assembler-arm.h#newcode43 src/arm/assem