Simplified CPU/CpuFeatures a bit.

Simplified CPU/CpuFeatures a bit.

This is a necessary intermediate step to disentangle the startup.

In the long run CPU and CpuFeatures should probably be merged,
and Serializer::enabled usage should be radically reduced, but
we're not there yet.

BUG=359977
LOG=y
R=bmeurer@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=21001

[Benedikt Meurer, 2014-04-28 08:09:19]

lgtm

[Sven Panne, 2014-04-28 08:18:53]

Committed patchset #1 manually as r21001 (presubmit successful).

[m.m.capewell, 2014-04-28 16:27:50]

On 2014/04/28 08:18:53, Sven Panne wrote:
> Committed patchset #1 manually as r21001 (presubmit successful).

I think this breaks ARM64 non-simulator builds, as at least
test/cctest/test-assembler-arm64.cc uses CPU::Setup().

[Sven Panne, 2014-04-29 06:26:54]

On 2014/04/28 16:27:50, m.m.capewell wrote:
> On 2014/04/28 08:18:53, Sven Panne wrote:
> > Committed patchset #1 manually as r21001 (presubmit successful).
> 
> I think this breaks ARM64 non-simulator builds, as at least
> test/cctest/test-assembler-arm64.cc uses CPU::Setup().

Hmmm, I am totally unable to test this, so it would be nice if you could check:

   * Can we simply remove that line?

   * If not: What exactly goes wrong? Does replacing it with
'CpuFeatures::Probe(false)' work?

Until we have bot coverage and/or less #if/#else/#fi horror, breaking things
will continue, I fear...

[m.m.capewell, 2014-04-29 09:24:17]

On 2014/04/29 06:26:54, Sven Panne wrote:
> On 2014/04/28 16:27:50, m.m.capewell wrote:
> > On 2014/04/28 08:18:53, Sven Panne wrote:
> > > Committed patchset #1 manually as r21001 (presubmit successful).
> > 
> > I think this breaks ARM64 non-simulator builds, as at least
> > test/cctest/test-assembler-arm64.cc uses CPU::Setup().
> 
> Hmmm, I am totally unable to test this, so it would be nice if you could
check:
> 
>    * Can we simply remove that line?
> 
>    * If not: What exactly goes wrong? Does replacing it with
> 'CpuFeatures::Probe(false)' work?
> 
> Until we have bot coverage and/or less #if/#else/#fi horror, breaking things
> will continue, I fear...

CpuFeatures::Probe(false) should be equivalent, if the function is visible from
that file. We'll submit a patch, as we can test it.

[m.m.capewell, 2014-04-29 09:42:40]

On 2014/04/29 09:24:17, m.m.capewell wrote:
> On 2014/04/29 06:26:54, Sven Panne wrote:
> > On 2014/04/28 16:27:50, m.m.capewell wrote:
> > > On 2014/04/28 08:18:53, Sven Panne wrote:
> > > > Committed patchset #1 manually as r21001 (presubmit successful).
> > > 
> > > I think this breaks ARM64 non-simulator builds, as at least
> > > test/cctest/test-assembler-arm64.cc uses CPU::Setup().
> > 
> > Hmmm, I am totally unable to test this, so it would be nice if you could
> check:
> > 
> >    * Can we simply remove that line?
> > 
> >    * If not: What exactly goes wrong? Does replacing it with
> > 'CpuFeatures::Probe(false)' work?
> > 
> > Until we have bot coverage and/or less #if/#else/#fi horror, breaking things
> > will continue, I fear...
> 
> CpuFeatures::Probe(false) should be equivalent, if the function is visible
from
> that file. We'll submit a patch, as we can test it.

Looks like Ulan has a fix already.

[ulan, 2014-04-29 09:45:34]

On 2014/04/29 09:42:40, m.m.capewell wrote:
> On 2014/04/29 09:24:17, m.m.capewell wrote:
> > On 2014/04/29 06:26:54, Sven Panne wrote:
> > > On 2014/04/28 16:27:50, m.m.capewell wrote:
> > > > On 2014/04/28 08:18:53, Sven Panne wrote:
> > > > > Committed patchset #1 manually as r21001 (presubmit successful).
> > > > 
> > > > I think this breaks ARM64 non-simulator builds, as at least
> > > > test/cctest/test-assembler-arm64.cc uses CPU::Setup().
> > > 
> > > Hmmm, I am totally unable to test this, so it would be nice if you could
> > check:
> > > 
> > >    * Can we simply remove that line?
> > > 
> > >    * If not: What exactly goes wrong? Does replacing it with
> > > 'CpuFeatures::Probe(false)' work?
> > > 
> > > Until we have bot coverage and/or less #if/#else/#fi horror, breaking
things
> > > will continue, I fear...
> > 
> > CpuFeatures::Probe(false) should be equivalent, if the function is visible
> from
> > that file. We'll submit a patch, as we can test it.
> 
> Looks like Ulan has a fix already.

Yep, I didn't see this before uploading a fix:
https://codereview.chromium.org/253873010/

[vincent.belliard, 2014-04-29 15:25:17]

https://codereview.chromium.org/258993002/diff/1/src/arm/assembler-arm.cc
File src/arm/assembler-arm.cc (left):

https://codereview.chromium.org/258993002/diff/1/src/arm/assembler-arm.cc#old...
src/arm/assembler-arm.cc:137: PrintFeatures();
Why did you removed this display?
It was very useful to check during the build that the snapshot was generated
with the good options.

[Sven Panne, 2014-04-30 06:11:15]

https://codereview.chromium.org/258993002/diff/1/src/arm/assembler-arm.cc
File src/arm/assembler-arm.cc (left):

https://codereview.chromium.org/258993002/diff/1/src/arm/assembler-arm.cc#old...
src/arm/assembler-arm.cc:137: PrintFeatures();
On 2014/04/29 15:25:17, vincent.belliard.arm wrote:
> Why did you removed this display?
> It was very useful to check during the build that the snapshot was generated
> with the good options.

It is not done for other platforms, it is printed with --help, anyway (twice
before), and I don't think the current approach of #ifdef'ing code just for ARM
(where the other PrintFeatures is residing) directly below src/ is a good idea,
this has to go away, too. ARM-specififc stuff should be below src/arm, the
current approach is a hack.