First set of replies...
http://codereview.chromium.org/1930/diff/1/9 File src/builtins-arm.cc (right): http://codereview.chromium.org/1930/diff/1/9#newcode67 Line 67: __ push(r1); // constructor function On 2008/09/11 21:33:13, Feng Qian wrote: > The constructor function pushed on the stack seems not used. Double check! It is used for example on line 78: __ ldr(r1, MemOperand(sp, kPointerSize)); http://codereview.chromium.org/1930/diff/1/9#newcode100 Line 100: // r3: number of arguments (smi-tagged) On 2008/09/11 21:33:13, Feng Qian wrote: > How about adding one more line comment here: > // r2: caller sp Fixed http://codereview.chromium.org/1930/diff/1/18 File src/codegen-arm.cc (right): http://codereview.chromium.org/1930/diff/1/18#newcode3876 Line 3876: ASSERT(args->length() == 1); On 2008/09/11 21:33:13, Feng Qian wrote: > Please update comment above function declaration. Outdated comment removed. http://codereview.chromium.org/1930/diff/1/8 File src/frames-arm.cc (right): http://codereview.chromium.org/1930/diff/1/8#newcode48 Line 48: return static_cast<StackFrame::Type>(Smi::cast(marker)->value()); On 2008/09/11 21:33:13, Feng Qian wrote: > Now this function looks exactly like the on in frame-ia32.cc. Can we move it to > frames.cc? This change is already unwieldy. Some of the cleanup should move to a follow up change. http://codereview.chromium.org/1930/diff/1/20 File src/macro-assembler-arm.cc (right): http://codereview.chromium.org/1930/diff/1/20#newcode299 Line 299: On 2008/09/11 21:33:13, Feng Qian wrote: > This part is a bit complicated. I can see unnecessary mov instructions in > different paths. For example, when both actual and expected are immediate but > not equal, there are two instructions that move actual.immediate() into r0. > Removed leftover code from the old calling convention. Thanks for spotting! http://codereview.chromium.org/1930/diff/1/20#newcode665 Line 665: Builtins::JavaScript id, bool* resolved) { On 2008/09/11 21:33:13, Feng Qian wrote: > Can this function be made that same as in IA32? (non-static). > > Part of code looks identical as the same function in IA32. You may want to > refactor the common code out. Factored the common code out into Builtins::GetCode(JavaScript id, bool* resolved). http://codereview.chromium.org/1930/diff/1/13 File src/simulator-arm.cc (right): http://codereview.chromium.org/1930/diff/1/13#newcode94 Line 94: OS::Abort(); On 2008/09/11 10:43:23, Kasper Lund wrote: > Is this intentional? Yes, this was just needed so that test.py does not wait until the timeout expires when hitting a stop instruction. Will be removed before putback. http://codereview.chromium.org/1930 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
