Updated file uploaded.  Comments addressed.

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),
On 2009/01/24 08:00:13, Kevin Millikin wrote:
> Since you're cleaning up this code, can you move the initializers to
one per
> line?

Done.

http://codereview.chromium.org/18669/diff/203/3#newcode1051
Line 1051: new DeferredInlinedSmiOperation(this, Token::SAR, smi_value,
On 2009/01/24 08:00:13, Kevin Millikin wrote:
> 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.

The masking with 0x1f is performed within the slow case, in runtime.cc,
matching what is done in the fast case here.  Comment added.

http://codereview.chromium.org/18669/diff/203/3#newcode1073
Line 1073: VirtualFrame::SpilledScope spilled_scope(this);
On 2009/01/24 08:00:13, Kevin Millikin wrote:
> Remove this spilled scope.

Done.

http://codereview.chromium.org/18669/diff/203/3#newcode1076
Line 1076: new DeferredInlinedSmiOperation(this, Token::SHR, smi_value,
On 2009/01/24 08:00:13, Kevin Millikin wrote:
> smi_value ==> Smi::FromInt(shift_value)

Not needed.  Commented.

http://codereview.chromium.org/18669

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

Reply via email to