As suggested I changed this to be a special case in the existing shift logic
(the special case where right is known to be a smi and we have SSE2
support).
http://codereview.chromium.org/955001/diff/3001/4001
File src/ia32/codegen-ia32.cc (right):
http://codereview.chromium.org/955001/diff/3001/4001#newcode1445
src/ia32/codegen-ia32.cc:1445: right->number_info().IsSmi()) {
On 2010/03/15 08:45:58, Lasse Reichstein wrote:
What about the case where left->number_info().IsSmi()? Could you skip
some of
the code in that case?
Done.
http://codereview.chromium.org/955001/diff/3001/4001#newcode1474
src/ia32/codegen-ia32.cc:1474: __ SmiUntag(answer.reg());
On 2010/03/15 08:45:58, Lasse Reichstein wrote:
Would it be worth not untagging if doing right shifts, and just
increase the
shift amount?
Probably not (you would have to test the increment of ecx for
overflowing).
Not relevant in new version.
http://codereview.chromium.org/955001/diff/3001/4001#newcode1514
src/ia32/codegen-ia32.cc:1514: __ test(answer.reg(),
Immediate(0xc0000000));
On 2010/03/15 08:45:58, Lasse Reichstein wrote:
This test is correct for SHR, but overly restrictive for the other
shifts (those
with signed 32-bit results).
For those do:
__ cmp(answer.reg(), Immediate(0xc0000000));
__ deferred->Branch(negative);
Using test instead would miss the negative smi results.
Also, please do a STATIC_ASSERT(kSmiValueSize == 31) to document what
you are
testing for (which are pretty likely for both ASR and SHL).
Not in new version
http://codereview.chromium.org/955001
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
