http://codereview.chromium.org/604054/diff/3001/1002 File src/arm/stub-cache-arm.cc (right):
http://codereview.chromium.org/604054/diff/3001/1002#newcode979 src/arm/stub-cache-arm.cc:979: Register scratch1 = r2; On 2010/02/15 10:58:50, Kevin Millikin wrote:
Why not call this something like "holder_reg"?
Done. http://codereview.chromium.org/604054/diff/3001/1002#newcode988 src/arm/stub-cache-arm.cc:988: // Get the receiver from the stack into r0. On 2010/02/15 10:58:50, Kevin Millikin wrote:
You should probably change the comments to not mention specific
registers. Done. http://codereview.chromium.org/604054/diff/3001/1002#newcode998 src/arm/stub-cache-arm.cc:998: CheckPrototypes(JSObject::cast(object), receiver, holder, On 2010/02/15 10:58:50, Kevin Millikin wrote:
It's not clear in this function that object can be cast to a JSObject.
If it's
a precondition of the function, change the type of the parameter to
JSObject*. This is a general issue on ComputeCallInterceptor and ComputeCallField, across all platforms, with a TODO and a bug filed already. This should be a separate change. There is code in the caller of this function that replaces object with the holder of value types, so the cast succeeds. http://codereview.chromium.org/604054/diff/3001/1002#newcode1002 src/arm/stub-cache-arm.cc:1002: reg = scratch1; On 2010/02/15 10:58:50, Kevin Millikin wrote:
Don't bother assigning to reg, it's (effectively) dead since you use
scratch1
(almost) consistently below. Change the one use of reg below and get
rid of
this assignment.
Done. http://codereview.chromium.org/604054/diff/3001/1002#newcode1014 src/arm/stub-cache-arm.cc:1014: JSFunction* function = lookup.GetConstantFunction(); On 2010/02/15 10:58:50, Kevin Millikin wrote:
Extra space after =. It seems strange to name this when it's only
used once in
the body and that use is so far from the name.
Done. http://codereview.chromium.org/604054/diff/3001/1002#newcode1018 src/arm/stub-cache-arm.cc:1018: __ push(scratch1); // Save the holder. On 2010/02/15 10:58:50, Kevin Millikin wrote:
__ push(holder_reg)
Done. http://codereview.chromium.org/604054/diff/3001/1002#newcode1050 src/arm/stub-cache-arm.cc:1050: // Save the name_ register across the call. On 2010/02/15 10:58:50, Kevin Millikin wrote:
This comment doesn't make sense (there is no "name_").
Done. http://codereview.chromium.org/604054/diff/3001/1002#newcode1063 src/arm/stub-cache-arm.cc:1063: // Restore the name_ register. On 2010/02/15 10:58:50, Kevin Millikin wrote:
This comment doesn't make sense (there is no "name_"). I'm not sure
there needs
to be a comment for __ pop(name_reg).
Done. http://codereview.chromium.org/604054 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
