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

http://codereview.chromium.org/18669/diff/203/3#newcode1087
Line 1087: if (shift_value < 2) {
On 2009/01/26 09:55:51, Kevin Millikin wrote:
> This needs a comment so that 2 doesn't seem so magic.

Done.

http://codereview.chromium.org/18669/diff/203/3#newcode1092
Line 1092: // tag result and store it in TOS (eax)
On 2009/01/26 09:55:51, Kevin Millikin wrote:
> Remove this comment.

Done.

http://codereview.chromium.org/18669/diff/203/3#newcode1112
Line 1112: new DeferredInlinedSmiOperation(this, Token::SHL, smi_value,
On 2009/01/26 09:55:51, Kevin Millikin wrote:
> Smi::FromInt(shift_value)?

Not needed.

http://codereview.chromium.org/18669/diff/203/3#newcode1124
Line 1124: } else if (shift_value > 1) {
On 2009/01/26 09:55:51, Kevin Millikin wrote:
> Is this an "else if" or a simple else (possibly with an assert that
shift_value
> > 1)?

It is really an else if, and the shift_value == 1 case is now commented.

http://codereview.chromium.org/18669/diff/203/3#newcode1127
Line 1127: // Convert int result to Smi, checking that it is in int
range.
On 2009/01/26 09:55:51, Kevin Millikin wrote:
> Is it possible to just shift the smi-tagged value, rather than
untagging,
> shifting, and retagging?

We have dropped untagging except in the shift 0 case, but shifting and
retagging need to be separate, for the Smi test to be combined with
retagging.

http://codereview.chromium.org/18669

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

Reply via email to