Updates refactoring of CPU feature detection

Updates refactoring of CPU feature detection
with fix that uses cpuid for Intel/Linux.

R=asiva@google.com

Committed: https://code.google.com/p/dart/source/detail?r=32980

[zra, 2014-02-20 21:33:12]

Ping. After this lands, it'll be easier to bring up the VM on beaglebone, etc.

[siva, 2014-02-21 19:28:55]

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu.h
File runtime/vm/cpu.h (right):

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu.h#newcode24
runtime/vm/cpu.h:24: 
extra blank line?

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu_arm.h
File runtime/vm/cpu_arm.h (right):

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu_arm.h#ne...
runtime/vm/cpu_arm.h:1: // Copyright (c) 2012, the Dart project authors.  Please
see the AUTHORS file
new file so 2014?

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu_ia32.cc
File runtime/vm/cpu_ia32.cc (right):

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu_ia32.cc#...
runtime/vm/cpu_ia32.cc:39: #define __ assembler.
I don't see any use of __ why is it defined here and undefed immediately
afterwards.

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu_ia32.h
File runtime/vm/cpu_ia32.h (right):

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu_ia32.h#n...
runtime/vm/cpu_ia32.h:1: // Copyright (c) 2012, the Dart project authors. 
Please see the AUTHORS file
ditto, 2014

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu_mips.h
File runtime/vm/cpu_mips.h (right):

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu_mips.h#n...
runtime/vm/cpu_mips.h:1: // Copyright (c) 2012, the Dart project authors. 
Please see the AUTHORS file
2014?

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu_x64.cc
File runtime/vm/cpu_x64.cc (right):

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu_x64.cc#n...
runtime/vm/cpu_x64.cc:41: #define __ assembler.
ditto comment about why __ is needed here?

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu_x64.h
File runtime/vm/cpu_x64.h (right):

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu_x64.h#ne...
runtime/vm/cpu_x64.h:1: // Copyright (c) 2012, the Dart project authors.  Please
see the AUTHORS file
2014

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuid.cc
File runtime/vm/cpuid.cc (right):

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuid.cc#new...
runtime/vm/cpuid.cc:1: // Copyright (c) 2012, the Dart project authors.  Please
see the AUTHORS file
2014

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuid.cc#new...
runtime/vm/cpuid.cc:41: char *id_string = new char[3 * sizeof(int32_t)];
probably you also need a CpuId::Cleanup() method that should be called from
Dart::Cleanup and id_string can be freed in that function.

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuid.h
File runtime/vm/cpuid.h (right):

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuid.h#newc...
runtime/vm/cpuid.h:1: // Copyright (c) 2012, the Dart project authors.  Please
see the AUTHORS file
2014

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuid.h#newc...
runtime/vm/cpuid.h:22: #endif
Why not move the implementation of these methods to the respective cpu_xxx.cc
files that way we won't have this #if defined(...) stuff here.

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuinfo.h
File runtime/vm/cpuinfo.h (right):

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuinfo.h#ne...
runtime/vm/cpuinfo.h:1: // Copyright (c) 2012, the Dart project authors.  Please
see the AUTHORS file
2014

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuinfo.h#ne...
runtime/vm/cpuinfo.h:57: static const char* ExtractFieldByString(const char*
field);
Document that the caller of this method is responsible to free the result
string.

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuinfo.h#ne...
runtime/vm/cpuinfo.h:59: // Returns the field describing the CPU model.
Document that the caller of this method should free the returned string

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuinfo_andr...
File runtime/vm/cpuinfo_android.cc (right):

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuinfo_andr...
runtime/vm/cpuinfo_android.cc:1: // Copyright (c) 2012, the Dart project
authors.  Please see the AUTHORS file
2014

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuinfo_linu...
File runtime/vm/cpuinfo_linux.cc (right):

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuinfo_linu...
runtime/vm/cpuinfo_linux.cc:55: }
As discussed offline we could enforce either CpuId::InitOnce or
ProcCpuInfo::InitOnce based on the architecture.

[zra, 2014-02-21 21:51:55]

Thanks!

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu.h
File runtime/vm/cpu.h (right):

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu.h#newcode24
runtime/vm/cpu.h:24: 
On 2014/02/21 19:28:55, siva wrote:
> extra blank line?

Removed.

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu_arm.h
File runtime/vm/cpu_arm.h (right):

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu_arm.h#ne...
runtime/vm/cpu_arm.h:1: // Copyright (c) 2012, the Dart project authors.  Please
see the AUTHORS file
On 2014/02/21 19:28:55, siva wrote:
> new file so 2014?

Done.

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu_ia32.cc
File runtime/vm/cpu_ia32.cc (right):

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu_ia32.cc#...
runtime/vm/cpu_ia32.cc:39: #define __ assembler.
On 2014/02/21 19:28:55, siva wrote:
> I don't see any use of __ why is it defined here and undefed immediately
> afterwards.

Removed.

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu_ia32.h
File runtime/vm/cpu_ia32.h (right):

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu_ia32.h#n...
runtime/vm/cpu_ia32.h:1: // Copyright (c) 2012, the Dart project authors. 
Please see the AUTHORS file
On 2014/02/21 19:28:55, siva wrote:
> ditto, 2014

Done.

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu_mips.h
File runtime/vm/cpu_mips.h (right):

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu_mips.h#n...
runtime/vm/cpu_mips.h:1: // Copyright (c) 2012, the Dart project authors. 
Please see the AUTHORS file
On 2014/02/21 19:28:55, siva wrote:
> 2014?

Done.

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu_x64.cc
File runtime/vm/cpu_x64.cc (right):

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu_x64.cc#n...
runtime/vm/cpu_x64.cc:41: #define __ assembler.
On 2014/02/21 19:28:55, siva wrote:
> ditto comment about why __ is needed here?

Removed.

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu_x64.h
File runtime/vm/cpu_x64.h (right):

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpu_x64.h#ne...
runtime/vm/cpu_x64.h:1: // Copyright (c) 2012, the Dart project authors.  Please
see the AUTHORS file
On 2014/02/21 19:28:55, siva wrote:
> 2014

Done.

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuid.cc
File runtime/vm/cpuid.cc (right):

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuid.cc#new...
runtime/vm/cpuid.cc:1: // Copyright (c) 2012, the Dart project authors.  Please
see the AUTHORS file
On 2014/02/21 19:28:55, siva wrote:
> 2014

Done.

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuid.cc#new...
runtime/vm/cpuid.cc:41: char *id_string = new char[3 * sizeof(int32_t)];
On 2014/02/21 19:28:55, siva wrote:
> probably you also need a CpuId::Cleanup() method that should be called from
> Dart::Cleanup and id_string can be freed in that function.

Done.

Also added cleanup code for ProcCpuInfo, and added a Cleanup call to CpuInfo, so
there is a generic Cleanup call we can make from Dart::Cleanup.

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuid.h
File runtime/vm/cpuid.h (right):

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuid.h#newc...
runtime/vm/cpuid.h:1: // Copyright (c) 2012, the Dart project authors.  Please
see the AUTHORS file
On 2014/02/21 19:28:55, siva wrote:
> 2014

Done.

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuid.h#newc...
runtime/vm/cpuid.h:22: #endif
On 2014/02/21 19:28:55, siva wrote:
> Why not move the implementation of these methods to the respective cpu_xxx.cc
> files that way we won't have this #if defined(...) stuff here.

As discussed, leaving here to avoid polluting arch-specific files.

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuinfo.h
File runtime/vm/cpuinfo.h (right):

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuinfo.h#ne...
runtime/vm/cpuinfo.h:1: // Copyright (c) 2012, the Dart project authors.  Please
see the AUTHORS file
On 2014/02/21 19:28:55, siva wrote:
> 2014

Done.

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuinfo.h#ne...
runtime/vm/cpuinfo.h:57: static const char* ExtractFieldByString(const char*
field);
On 2014/02/21 19:28:55, siva wrote:
> Document that the caller of this method is responsible to free the result
> string.

Done.

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuinfo.h#ne...
runtime/vm/cpuinfo.h:59: // Returns the field describing the CPU model.
On 2014/02/21 19:28:55, siva wrote:
> Document that the caller of this method should free the returned string

Done.

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuinfo_andr...
File runtime/vm/cpuinfo_android.cc (right):

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuinfo_andr...
runtime/vm/cpuinfo_android.cc:1: // Copyright (c) 2012, the Dart project
authors.  Please see the AUTHORS file
On 2014/02/21 19:28:55, siva wrote:
> 2014

Done.

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuinfo_linu...
File runtime/vm/cpuinfo_linux.cc (right):

https://codereview.chromium.org/136303012/diff/840001/runtime/vm/cpuinfo_linu...
runtime/vm/cpuinfo_linux.cc:55: }
On 2014/02/21 19:28:55, siva wrote:
> As discussed offline we could enforce either CpuId::InitOnce or
> ProcCpuInfo::InitOnce based on the architecture.

Done.

[siva, 2014-02-21 22:02:14]

lgtm

[zra, 2014-02-24 19:28:47]

Committed patchset #11 manually as r32980 (presubmit successful).