Rework v8_target_arch / target_cpu approach to use v8_target_cpu.

Rework v8_target_arch / target_cpu approach to use v8_target_cpu.

This CL reworks how we get architecture-specific build args configured
correctly so that they will be set when needed for either the
current_cpu in the build toolchain *or* when needed to compile v8
correctly. Eventually we should rework this so that it is a generic
concept and not v8 specific.

R=brettw@chromium.org, thakis@chromium.org, jochen@chromium.org, machenbach@chromium.org
BUG=619503, 620527
Committed: https://crrev.com/909e931c942774a052b493bd0061325502373f3a
Cr-Commit-Position: refs/heads/master@{#400219}

[Dirk Pranke, 2016-06-16 01:21:17]

dpranke@chromium.org changed reviewers:
+ machenbach@chromium.org

[Dirk Pranke, 2016-06-16 01:21:33]

Description was changed from

==========
Rework v8_target_arch / target_cpu approach to use v8_target_cpu.

This CL reworks how we get architecture-specific build args configured
correctly so that they will be set when needed for either the
current_cpu in the build toolchain *or* when needed to compile v8
correctly. Eventually we should rework this so that it is a generic
concept and not v8 specific.

R=brettw@chromium.org, thakis@chromium.org, jochen@chromium.org
BUG=619503, 620527
==========

to

==========
Rework v8_target_arch / target_cpu approach to use v8_target_cpu.

This CL reworks how we get architecture-specific build args configured
correctly so that they will be set when needed for either the
current_cpu in the build toolchain *or* when needed to compile v8
correctly. Eventually we should rework this so that it is a generic
concept and not v8 specific.

R=brettw@chromium.org, thakis@chromium.org, jochen@chromium.org,
machenbach@chromium.org
BUG=619503, 620527
==========

[Michael Achenbach, 2016-06-16 07:49:55]

lgtm

https://codereview.chromium.org/2070653003/diff/1/build/config/arm.gni
File build/config/arm.gni (right):

https://codereview.chromium.org/2070653003/diff/1/build/config/arm.gni#newcode90
build/config/arm.gni:90: } else if (current_cpu == "arm64" || v8_target_cpu ==
"arm64") {
ocd nit: maybe turn the conditions around like for arm above:
v8_target_cpu == "arm64" || current_cpu == "arm64"

https://codereview.chromium.org/2070653003/diff/1/build/config/v8_target_cpu.gni
File build/config/v8_target_cpu.gni (right):

https://codereview.chromium.org/2070653003/diff/1/build/config/v8_target_cpu....
build/config/v8_target_cpu.gni:19: v8_target_cpu = target_cpu
This logic:
https://cs.chromium.org/chromium/src/v8/gni/v8.gni?l=24

should move here too. Maybe with an extra check that target_cpu==x64, otherwise
v8_target_cpu arm64 makes no sense.

[Nico, 2016-06-16 08:17:28]

lgtm

https://codereview.chromium.org/2070653003/diff/1/build/config/arm.gni
File build/config/arm.gni (right):

https://codereview.chromium.org/2070653003/diff/1/build/config/arm.gni#newcode11
build/config/arm.gni:11: if (v8_target_cpu == "arm" || current_cpu == "arm") {
ubernit: maybe swap the operands to || to match the mips file (and to put the
common case first)

[Dirk Pranke, 2016-06-16 17:35:05]

https://codereview.chromium.org/2070653003/diff/1/build/config/arm.gni
File build/config/arm.gni (right):

https://codereview.chromium.org/2070653003/diff/1/build/config/arm.gni#newcode11
build/config/arm.gni:11: if (v8_target_cpu == "arm" || current_cpu == "arm") {
On 2016/06/16 08:17:28, Nico (traveling...slow) wrote:
> ubernit: maybe swap the operands to || to match the mips file (and to put the
> common case first)

Acknowledged.

https://codereview.chromium.org/2070653003/diff/1/build/config/arm.gni#newcode90
build/config/arm.gni:90: } else if (current_cpu == "arm64" || v8_target_cpu ==
"arm64") {
On 2016/06/16 07:49:55, Michael Achenbach wrote:
> ocd nit: maybe turn the conditions around like for arm above:
> v8_target_cpu == "arm64" || current_cpu == "arm64" 

Ack.

https://codereview.chromium.org/2070653003/diff/1/build/config/v8_target_cpu.gni
File build/config/v8_target_cpu.gni (right):

https://codereview.chromium.org/2070653003/diff/1/build/config/v8_target_cpu....
build/config/v8_target_cpu.gni:19: v8_target_cpu = target_cpu
On 2016/06/16 07:49:55, Michael Achenbach wrote:
> This logic:
> https://cs.chromium.org/chromium/src/v8/gni/v8.gni?l=24
> 
> should move here too. Maybe with an extra check that target_cpu==x64,
otherwise
> v8_target_cpu arm64 makes no sense.

Done, in the interest of moving forward, but I was wondering about this ... does
this mean that the normal x64 JIT path doesn't work correctly under MSAN? That
seems bad ...

[Dirk Pranke, 2016-06-16 17:36:47]

The CQ bit was checked by dpranke@chromium.org

[Dirk Pranke, 2016-06-16 17:36:48]

The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org,
machenbach@chromium.org
Link to the patchset: https://codereview.chromium.org/2070653003/#ps20001
(title: "update w/ review feedback")

[commit-bot: I haz the power, 2016-06-16 17:37:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2070653003/20001

[commit-bot: I haz the power, 2016-06-16 18:43:48]

Description was changed from

==========
Rework v8_target_arch / target_cpu approach to use v8_target_cpu.

This CL reworks how we get architecture-specific build args configured
correctly so that they will be set when needed for either the
current_cpu in the build toolchain *or* when needed to compile v8
correctly. Eventually we should rework this so that it is a generic
concept and not v8 specific.

R=brettw@chromium.org, thakis@chromium.org, jochen@chromium.org,
machenbach@chromium.org
BUG=619503, 620527
==========

to

==========
Rework v8_target_arch / target_cpu approach to use v8_target_cpu.

This CL reworks how we get architecture-specific build args configured
correctly so that they will be set when needed for either the
current_cpu in the build toolchain *or* when needed to compile v8
correctly. Eventually we should rework this so that it is a generic
concept and not v8 specific.

R=brettw@chromium.org, thakis@chromium.org, jochen@chromium.org,
machenbach@chromium.org
BUG=619503, 620527
==========

[commit-bot: I haz the power, 2016-06-16 18:43:49]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-16 18:46:56]

Description was changed from

==========
Rework v8_target_arch / target_cpu approach to use v8_target_cpu.

This CL reworks how we get architecture-specific build args configured
correctly so that they will be set when needed for either the
current_cpu in the build toolchain *or* when needed to compile v8
correctly. Eventually we should rework this so that it is a generic
concept and not v8 specific.

R=brettw@chromium.org, thakis@chromium.org, jochen@chromium.org,
machenbach@chromium.org
BUG=619503, 620527
==========

to

==========
Rework v8_target_arch / target_cpu approach to use v8_target_cpu.

This CL reworks how we get architecture-specific build args configured
correctly so that they will be set when needed for either the
current_cpu in the build toolchain *or* when needed to compile v8
correctly. Eventually we should rework this so that it is a generic
concept and not v8 specific.

R=brettw@chromium.org, thakis@chromium.org, jochen@chromium.org,
machenbach@chromium.org
BUG=619503, 620527

Committed: https://crrev.com/909e931c942774a052b493bd0061325502373f3a
Cr-Commit-Position: refs/heads/master@{#400219}
==========

[commit-bot: I haz the power, 2016-06-16 18:46:57]

Patchset 2 (id:??) landed as
https://crrev.com/909e931c942774a052b493bd0061325502373f3a
Cr-Commit-Position: refs/heads/master@{#400219}

[Michael Achenbach, 2016-06-17 06:51:41]

>
https://codereview.chromium.org/2070653003/diff/1/build/config/v8_target_cpu.gni
> File build/config/v8_target_cpu.gni (right):
> 
>
https://codereview.chromium.org/2070653003/diff/1/build/config/v8_target_cpu....
> build/config/v8_target_cpu.gni:19: v8_target_cpu = target_cpu
> On 2016/06/16 07:49:55, Michael Achenbach wrote:
> > This logic:
> > https://cs.chromium.org/chromium/src/v8/gni/v8.gni?l=24
> > 
> > should move here too. Maybe with an extra check that target_cpu==x64,
> otherwise
> > v8_target_cpu arm64 makes no sense.
> 
> Done, in the interest of moving forward, but I was wondering about this ...
does
> this mean that the normal x64 JIT path doesn't work correctly under MSAN? That
> seems bad ...

I think it works, but it doesn't use a simulator and therefore doesn't
instrument and check the jitted code. The real power comes from msan +
simulator. We'd need an x64 on x64 simulator, but we don't have one.

[Dirk Pranke, 2016-06-17 15:56:46]

On 2016/06/17 06:51:41, Michael Achenbach wrote:
> >
>
https://codereview.chromium.org/2070653003/diff/1/build/config/v8_target_cpu.gni
> > File build/config/v8_target_cpu.gni (right):
> > 
> >
>
https://codereview.chromium.org/2070653003/diff/1/build/config/v8_target_cpu....
> > build/config/v8_target_cpu.gni:19: v8_target_cpu = target_cpu
> > On 2016/06/16 07:49:55, Michael Achenbach wrote:
> > > This logic:
> > > https://cs.chromium.org/chromium/src/v8/gni/v8.gni?l=24
> > > 
> > > should move here too. Maybe with an extra check that target_cpu==x64,
> > otherwise
> > > v8_target_cpu arm64 makes no sense.
> > 
> > Done, in the interest of moving forward, but I was wondering about this ...
> does
> > this mean that the normal x64 JIT path doesn't work correctly under MSAN?
That
> > seems bad ...
> 
> I think it works, but it doesn't use a simulator and therefore doesn't
> instrument and check the jitted code. The real power comes from msan +
> simulator. We'd need an x64 on x64 simulator, but we don't have one.

Ah, makes sense.