It needs just a bit of clean up. LGTM if comments are addressed.
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; Why not call this something like "holder_reg"? http://codereview.chromium.org/604054/diff/3001/1002#newcode988 src/arm/stub-cache-arm.cc:988: // Get the receiver from the stack into r0. You should probably change the comments to not mention specific registers. http://codereview.chromium.org/604054/diff/3001/1002#newcode998 src/arm/stub-cache-arm.cc:998: CheckPrototypes(JSObject::cast(object), receiver, holder, 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*. http://codereview.chromium.org/604054/diff/3001/1002#newcode1002 src/arm/stub-cache-arm.cc:1002: reg = scratch1; 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. http://codereview.chromium.org/604054/diff/3001/1002#newcode1014 src/arm/stub-cache-arm.cc:1014: JSFunction* function = lookup.GetConstantFunction(); 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. http://codereview.chromium.org/604054/diff/3001/1002#newcode1018 src/arm/stub-cache-arm.cc:1018: __ push(scratch1); // Save the holder. __ push(holder_reg) http://codereview.chromium.org/604054/diff/3001/1002#newcode1023 src/arm/stub-cache-arm.cc:1023: scratch1, holder_reg http://codereview.chromium.org/604054/diff/3001/1002#newcode1028 src/arm/stub-cache-arm.cc:1028: __ pop(scratch1); // Restore the holder. holder_reg http://codereview.chromium.org/604054/diff/3001/1002#newcode1050 src/arm/stub-cache-arm.cc:1050: // Save the name_ register across the call. This comment doesn't make sense (there is no "name_"). http://codereview.chromium.org/604054/diff/3001/1002#newcode1063 src/arm/stub-cache-arm.cc:1063: // Restore the name_ register. This comment doesn't make sense (there is no "name_"). I'm not sure there needs to be a comment for __ pop(name_reg). http://codereview.chromium.org/604054 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
