Thanks for the comments!
http://codereview.chromium.org/146029/diff/2001/2006 File src/x64/builtins-x64.cc (right): http://codereview.chromium.org/146029/diff/2001/2006#newcode205 Line 205: __ EnterConstructFrame(); On 2009/06/24 07:55:57, William Hesse wrote: > What is the calling convention of this function on entry, and also after > EnterConstructFrame? rax: number of arguments rdi: constructor function http://codereview.chromium.org/146029/diff/2001/2006#newcode227 Line 227: __ movq(rbx, rax); // store result in ebx On 2009/06/24 07:55:57, William Hesse wrote: > rbx, not ebx Done. http://codereview.chromium.org/146029/diff/2001/2006#newcode230 Line 230: // ebx: newly allocated object On 2009/06/24 07:55:57, William Hesse wrote: > rbx Done. http://codereview.chromium.org/146029/diff/2001/2006#newcode253 Line 253: __ push(Operand(rbx, rcx, times_pointer_size, 0)); On 2009/06/24 07:55:57, William Hesse wrote: > Isn't there a three-argument version of Operand(Register, Register, > ScaleFactor)? If not, we should create one, I think. There isn't no, and I'm not sure it adds much to have it. http://codereview.chromium.org/146029/diff/2001/2006#newcode276 Line 276: __ CmpInstanceType(rcx, FIRST_JS_OBJECT_TYPE); On 2009/06/24 07:55:57, William Hesse wrote: > CmpInstanceType can now be written inline as a single instruction, > cmpb(Operand(rcx, typeOffset), Immediate(FIRST_JS_OBJECT_TYPE)) > Update CmpInstanceType, or else write instructions directly. > I have just put a bunch of these in - maybe they should all be changed to > CmpInstanceType(). CmpInstanceType does the one instruction compare. Here I should use CompareObjectType though to take care of the map load as well. http://codereview.chromium.org/146029/diff/2001/2008 File src/x64/codegen-x64.cc (right): http://codereview.chromium.org/146029/diff/2001/2008#newcode1403 Line 1403: Result temp = allocator()->Allocate(); On 2009/06/24 07:55:57, William Hesse wrote: > Use kScratchRegister here. Done. http://codereview.chromium.org/146029/diff/2001/2008#newcode1647 Line 1647: __ testl(value.reg(), Immediate(kSmiTagMask)); On 2009/06/24 07:55:57, William Hesse wrote: > We have many of these throughout the code, and have been using testq for them. > Since the high bits of the mask are zero, it doesn't matter which we use, but > testl can lead to a shorter encoding, I suppose. We should be consistent, for > now. If you plan to replacing these throughout all the code, that is OK with > me. We were using it inconsistently before. There was a mix of the two. I have changed it to consistently use testl in all the x64 files. http://codereview.chromium.org/146029 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
