Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(181)

Issue 10990028: Add code for CPU feature detection and use it to detect SSE3 and SSE 4.1. (Closed)

Created:
8 years, 2 months ago by Florian Schneider
Modified:
8 years, 2 months ago
Reviewers:
srdjan
CC:
reviews_dartlang.org, vm-dev_dartlang.org, siva
Visibility:
Public.

Description

Add code for CPU feature detection and use it to detect SSE3 and SSE 4.1. The Dart VM now requires at least SSE3 to build and run. There is a new flag --use-sse41 (default:true) to disable the use of SSE 4.1 features manually. Committed: https://code.google.com/p/dart/source/detail?r=12897

Patch Set 1 #

Patch Set 2 : simplified, because there is no code in snapshots #

Total comments: 6

Patch Set 3 : addressed comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+175 lines, -4 lines) Patch
M runtime/bin/gen_snapshot.cc View 1 1 chunk +4 lines, -1 line 0 comments Download
M runtime/bin/main.cc View 1 chunk +5 lines, -3 lines 0 comments Download
M runtime/vm/assembler_ia32.h View 2 chunks +20 lines, -0 lines 0 comments Download
M runtime/vm/assembler_ia32.cc View 1 2 2 chunks +59 lines, -0 lines 0 comments Download
M runtime/vm/assembler_x64.h View 1 2 2 chunks +20 lines, -0 lines 0 comments Download
M runtime/vm/assembler_x64.cc View 1 2 2 chunks +62 lines, -0 lines 0 comments Download
M runtime/vm/dart.cc View 1 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Florian Schneider
Is there a way currently to detect if the VM is creating a snapshot? I ...
8 years, 2 months ago (2012-09-25 15:52:14 UTC) #1
Florian Schneider
Uploaded new version with Srdjan's feedback addressed.
8 years, 2 months ago (2012-09-25 17:08:35 UTC) #2
srdjan
LGTM with comments https://codereview.chromium.org/10990028/diff/9001/runtime/bin/gen_snapshot.cc File runtime/bin/gen_snapshot.cc (right): https://codereview.chromium.org/10990028/diff/9001/runtime/bin/gen_snapshot.cc#newcode304 runtime/bin/gen_snapshot.cc:304: return 255; Why not exit(255) as ...
8 years, 2 months ago (2012-09-25 17:12:25 UTC) #3
Florian Schneider
8 years, 2 months ago (2012-09-26 09:03:16 UTC) #4
https://codereview.chromium.org/10990028/diff/9001/runtime/bin/gen_snapshot.cc
File runtime/bin/gen_snapshot.cc (right):

https://codereview.chromium.org/10990028/diff/9001/runtime/bin/gen_snapshot.c...
runtime/bin/gen_snapshot.cc:304: return 255;
On 2012/09/25 17:12:25, srdjan wrote:
> Why not exit(255) as below?

Because I copied it from above where return 255 is used :)
return seems cleaner to me since it also invokes destructors of stack-allocated
objects (e.g. CommandLineOptions)

I don't think that it matters here. exit() is used in many functions in case of
an error, so it would mean changing a lot to get rid of it.

https://codereview.chromium.org/10990028/diff/9001/runtime/vm/assembler_x64.cc
File runtime/vm/assembler_x64.cc (right):

https://codereview.chromium.org/10990028/diff/9001/runtime/vm/assembler_x64.c...
runtime/vm/assembler_x64.cc:46: // Get feature information in ECX:EDX and return
it in RAX.
On 2012/09/25 17:12:25, srdjan wrote:
> RCX, RDX ?

That is intentional according to the spec: cpuid only affects the 32-bit half
EAX, EBX, ECX, EDX also in 64-bit mode. I'll add a comment.

https://codereview.chromium.org/10990028/diff/9001/runtime/vm/assembler_x64.h
File runtime/vm/assembler_x64.h (right):

https://codereview.chromium.org/10990028/diff/9001/runtime/vm/assembler_x64.h...
runtime/vm/assembler_x64.h:287: static bool initialized_;
On 2012/09/25 17:12:25, srdjan wrote:
> in #ifdef DEBUG as on ia32?

Done.

Powered by Google App Engine
This is Rietveld 408576698