Chromium Code Reviews
Help | Chromium Project | Sign in
(475)

Issue 197057: Use SSE2 instructions when available on ia32 platform. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 7 months ago by William Hesse
Modified:
2 years, 11 months ago
CC:
v8-dev_googlegroups.com
Visibility:
Public.

Description

Use SSE2 instructions when available on ia32 platform.

Committed: http://code.google.com/p/v8/source/detail?r=2868

Patch Set 1 #

Total comments: 4

Patch Set 2 : '' #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+178 lines, -54 lines) Lint Patch
M src/ia32/assembler-ia32.h View 1 2 chunks +3 lines, -0 lines 0 comments 0 errors Download
M src/ia32/assembler-ia32.cc View 1 3 chunks +22 lines, -4 lines 0 comments 0 errors Download
M src/ia32/codegen-ia32.cc View 1 4 chunks +153 lines, -50 lines 4 comments 0 errors Download
Trybot results:
Commit:

Messages

Total messages: 5
William Hesse
4 years, 7 months ago #1
Erik Corry
LGTM We should ensure that SSE2 and CMOV are always enabled on MacOS, even if ...
4 years, 7 months ago #2
William Hesse
http://codereview.chromium.org/197057/diff/1/2 File src/ia32/codegen-ia32.cc (right): http://codereview.chromium.org/197057/diff/1/2#newcode7428 Line 7428: // Jump to builtin for NaN. On 2009/09/09 ...
4 years, 7 months ago #3
Mark Mentovai
I'm familiar with x86 but much less so with V8 internals, so these questions may ...
4 years, 7 months ago #4
William Hesse
4 years, 7 months ago #5
http://codereview.chromium.org/197057/diff/4001/4002
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/197057/diff/4001/4002#newcode6745
Line 6745: FloatingPointHelper::CheckFloatOperands(masm, &call_runtime, ebx);
On 2009/09/09 15:09:55, Mark Mentovai wrote:
> Shouldn't this be done for both SSE2 and x87, or should there be a
> CheckSse2Operands?

The checking and the loading are combined into one function in LoadSse2Operands,
so that we can be more efficient.  The Smi tag is only checked once.  This is
documented in the declaration of LoadSse2Operands().

http://codereview.chromium.org/197057/diff/4001/4002#newcode6746
Line 6746: // Allocate a heap number, if needed.
On 2009/09/09 15:09:55, Mark Mentovai wrote:
> Is there a reason that this is done before the x87 operations in this branch,
> but after the SSE2 operations in the branch above?
> 
This code has to go after the check, but before the store of the result, so it
is enclosed by code that depends on whether we use SSE2.  It also needs to have
the values still in the eax and edx registers.  Most of the common code has been
factored out into AllocateHeapNumber() already.

So either we need to put it in a separate function, or we need to end the
use_sse2 scope, run the common code, and the n run a test and potentially enter
a new sse2 scope again.  This doesn't seem worth it.
> Is there a way to share this duplicated code between the two branches?
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6