Initial round of comments.  I have not looked closely at the whole thing
yet so there will be more.


http://codereview.chromium.org/18669/diff/203/3
File src/codegen-ia32.cc (right):

http://codereview.chromium.org/18669/diff/203/3#newcode806
Line 806: DeferredCode(generator), op_(op), value_(value),
Since you're cleaning up this code, can you move the initializers to one
per line?

http://codereview.chromium.org/18669/diff/203/3#newcode1048
Line 1048: VirtualFrame::SpilledScope spilled_scope(this);
Remove this spilled scope.

http://codereview.chromium.org/18669/diff/203/3#newcode1051
Line 1051: new DeferredInlinedSmiOperation(this, Token::SAR, smi_value,
smi_value is not the Smi tagged version of shift_value.  It seems like
you have to pass Smi::FromInt(shift_value) to match the previous code.

http://codereview.chromium.org/18669/diff/203/3#newcode1073
Line 1073: VirtualFrame::SpilledScope spilled_scope(this);
Remove this spilled scope.

http://codereview.chromium.org/18669/diff/203/3#newcode1076
Line 1076: new DeferredInlinedSmiOperation(this, Token::SHR, smi_value,
smi_value ==> Smi::FromInt(shift_value)

http://codereview.chromium.org/18669

--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to