[gn] Add gn configs for mips to match old behavior

[gn] Add gn configs for mips to match old behavior

BUG=chromium:474921
NOTRY=true

Committed: https://crrev.com/a482ab5c82b46d9faeb3950e8fbb68a6e06f9a14
Cr-Commit-Position: refs/heads/master@{#39369}

[Michael Achenbach, 2016-09-09 09:46:53]

Description was changed from

==========
[gn] Add gn configs for mips to match old behavior

BUG=
==========

to

==========
[gn] Add gn configs for mips to match old behavior

BUG=chromium:474921
NOTRY=true
==========

[Michael Achenbach, 2016-09-09 09:46:59]

Description was changed from

==========
[gn] Add gn configs for mips to match old behavior

BUG=chromium:474921
NOTRY=true
==========

to

==========
[gn] Add gn configs for mips to match old behavior

BUG=chromium:474921
==========

[Michael Achenbach, 2016-09-09 09:54:26]

machenbach@chromium.org changed reviewers:
+ jochen@chromium.org, vogelheim@chromium.org

[Michael Achenbach, 2016-09-09 09:54:27]

PTAL

https://codereview.chromium.org/2322163003/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2322163003/diff/1/BUILD.gn#newcode242
BUILD.gn:242: (v8_current_cpu == "mipsel" || v8_current_cpu == "mips64el")) {
We set this for target simulator build in gyp here:
https://cs.chromium.org/chromium/src/v8/gypfiles/toolchain.gypi?q=_MIPS_TARGE...

Does this matter at all for snapshot generation?

_MIPS_TARGET_HW seems to be dead code.

https://codereview.chromium.org/2322163003/diff/1/infra/mb/mb_config.pyl
File infra/mb/mb_config.pyl (right):

https://codereview.chromium.org/2322163003/diff/1/infra/mb/mb_config.pyl#newc...
infra/mb/mb_config.pyl:556: 'target_cpu="x86" v8_target_cpu="mipsel"
mips_arch_variant="r2"',
Chromium uses r1 as default while we default to r2 in stand-alone builds in gyp:
https://cs.chromium.org/chromium/src/v8/gypfiles/standalone.gypi?l=445

Right now, the best way to express v8 stand-alone is to override the flags right
here.

[vogelheim, 2016-09-09 10:56:37]

lgtm

https://codereview.chromium.org/2322163003/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2322163003/diff/1/BUILD.gn#newcode242
BUILD.gn:242: (v8_current_cpu == "mipsel" || v8_current_cpu == "mips64el")) {
On 2016/09/09 09:54:27, machenbach (slow) wrote:
> We set this for target simulator build in gyp here:
>
https://cs.chromium.org/chromium/src/v8/gypfiles/toolchain.gypi?q=_MIPS_TARGE...
> 
> Does this matter at all for snapshot generation?
> 
> _MIPS_TARGET_HW seems to be dead code.

Might matter.

(This affects code generation, and thus might affect the code that ends up in
the snapshot. So if the snapshot code will later be run in the simulator (or
not), this might make a difference.)

[Michael Achenbach, 2016-09-09 12:32:24]

https://codereview.chromium.org/2322163003/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2322163003/diff/1/BUILD.gn#newcode241
BUILD.gn:241: if (target_cpu != v8_target_cpu &&
CC also Dirk. Want to make sure that this expresses the right thing.

target_cpu != v8_target_cpu makes sure we have a simulator build as the target.
v8_current_cpu == "mipsel" e.g. makes sure we simulate mipsel.

I assume if I had used current_cpu != v8_current_cpu, it would also match a
mipsel simulator build in the snapshot toolchain on the host when we
crosscompile (which would be wrong).

[Dirk Pranke, 2016-09-09 20:00:45]

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

[Dirk Pranke, 2016-09-09 20:00:46]

https://codereview.chromium.org/2322163003/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2322163003/diff/1/BUILD.gn#newcode241
BUILD.gn:241: if (target_cpu != v8_target_cpu &&
I'm not sure I understand the intent of this block. You want the define set when
doing a simulator build but *not* when generating the snapshot for the
simulator?

[Michael Achenbach, 2016-09-12 08:11:29]

https://codereview.chromium.org/2322163003/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2322163003/diff/1/BUILD.gn#newcode241
BUILD.gn:241: if (target_cpu != v8_target_cpu &&
On 2016/09/09 20:00:45, Dirk Pranke wrote:
> I'm not sure I understand the intent of this block. You want the define set
when
> doing a simulator build but *not* when generating the snapshot for the
> simulator?

Close. We want to set the define when the _target_ is a simulator build. We also
want to set it for creating the snapshot for such a build.

We don't want to set it when making the snapshot for a cross-compile target.

Making snapshot for cross-compile and other simulator builds can't be
distinguished by the toolchain. In both cases we'd have e.g. current_cpu="x86"
and v8_current_cpu="mipsel". The only way to distinguish I see is using target_X
for comparison.

This is simulating the old gyp behavior expressed with the _toolset=="target"
condition. E.g. here:
https://cs.chromium.org/chromium/src/v8/gypfiles/toolchain.gypi?q=_MIPS_TARGE...

[Dirk Pranke, 2016-09-12 21:19:44]

https://codereview.chromium.org/2322163003/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2322163003/diff/1/BUILD.gn#newcode241
BUILD.gn:241: if (target_cpu != v8_target_cpu &&
Got it, right.

Maybe create a separate top-level variable (not declare_arg(), just a regular
variable) called is_simulator_build or something, just to be a bit clearer.

[Michael Achenbach, 2016-09-13 07:12:56]

https://codereview.chromium.org/2322163003/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2322163003/diff/1/BUILD.gn#newcode241
BUILD.gn:241: if (target_cpu != v8_target_cpu &&
On 2016/09/12 21:19:44, Dirk Pranke wrote:
> Got it, right.
> 
> Maybe create a separate top-level variable (not declare_arg(), just a regular
> variable) called is_simulator_build or something, just to be a bit clearer.

Done

[Michael Achenbach, 2016-09-13 07:15:01]

The CQ bit was checked by machenbach@chromium.org

[Michael Achenbach, 2016-09-13 07:15:02]

The patchset sent to the CQ was uploaded after l-g-t-m from
vogelheim@chromium.org
Link to the patchset: https://codereview.chromium.org/2322163003/#ps20001
(title: "Review")

[commit-bot: I haz the power, 2016-09-13 07:15:11]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Michael Achenbach, 2016-09-13 07:25:57]

The CQ bit was unchecked by machenbach@chromium.org

[Michael Achenbach, 2016-09-13 07:30:03]

Description was changed from

==========
[gn] Add gn configs for mips to match old behavior

BUG=chromium:474921
==========

to

==========
[gn] Add gn configs for mips to match old behavior

BUG=chromium:474921
NOTRY=true
==========

[Michael Achenbach, 2016-09-13 07:30:12]

The CQ bit was checked by machenbach@chromium.org

[Michael Achenbach, 2016-09-13 07:30:13]

The patchset sent to the CQ was uploaded after l-g-t-m from
vogelheim@chromium.org
Link to the patchset: https://codereview.chromium.org/2322163003/#ps60001
(title: "...")

[commit-bot: I haz the power, 2016-09-13 07:30:20]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-13 07:32:46]

Description was changed from

==========
[gn] Add gn configs for mips to match old behavior

BUG=chromium:474921
NOTRY=true
==========

to

==========
[gn] Add gn configs for mips to match old behavior

BUG=chromium:474921
NOTRY=true
==========

[commit-bot: I haz the power, 2016-09-13 07:32:47]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-13 07:33:19]

Description was changed from

==========
[gn] Add gn configs for mips to match old behavior

BUG=chromium:474921
NOTRY=true
==========

to

==========
[gn] Add gn configs for mips to match old behavior

BUG=chromium:474921
NOTRY=true

Committed: https://crrev.com/a482ab5c82b46d9faeb3950e8fbb68a6e06f9a14
Cr-Commit-Position: refs/heads/master@{#39369}
==========

[commit-bot: I haz the power, 2016-09-13 07:33:20]

Patchset 4 (id:??) landed as
https://crrev.com/a482ab5c82b46d9faeb3950e8fbb68a6e06f9a14
Cr-Commit-Position: refs/heads/master@{#39369}