Land chromium-side work to clean up handling of v8_target_cpu in the GN build.

build/config/BUILDCONFIG.gn

 # Copyright (c) 2013 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 # =============================================================================
 # PLATFORM SELECTION
 # =============================================================================
 #
 # There are two main things to set: "os" and "cpu". The "toolchain" is the name
 # of the GN thing that encodes combinations of these things.
@@ skipping 124 matching lines @@
              current_os == "linux" || current_os == "chromeos"
 
   # Allows the path to a custom target toolchain to be injected as a single
   # argument, and set as the default toolchain.
   custom_toolchain = ""
 
   # This should not normally be set as a build argument.  It's here so that
   # every toolchain can pass through the "global" value via toolchain_args().
   host_toolchain = ""
 
+  # This arg is used when we want to tell the JIT-code-generating v8 code
+  # that we want to have it generate code for an architecture that is different
+  # than the architecture that v8 will actually run on, and that we will
+  # then run the code under an emulator. For example, we might run v8
+  # on x86, but generate arm code and run that under emulation.
+  #
+  # This arg is defined here rather than in the v8 project because we want
+  # some of the common architecture-specific args (like arm_float_abi or
+  # mips_arch_variant) to be set to their defaults either if the current_cpu

[Michael Achenbach, 2016/07/07 07:24:22]

nit: For consistency, maybe write either "current_cpu or v8_current_cpu" (like
it's implemented) or "target_cpu or v8_target_cpu" (might be easier to
understand).

[Dirk Pranke, 2016/07/07 17:28:12]

Will do (using target_cpu).

+  # applies *or* if the v8_target_cpu applies.
+  v8_target_cpu = ""
+
+  # Compile for Memory Sanitizer to find uninitialized reads.
+  #
+  # TODO: This is here so that we can set a good default for v8_target_cpu
+  # when running under the sanitizer, but this is something of a layering
+  # violation and it would be nice if there was some other way to do this.
+  is_msan = false

[Dirk Pranke, 2016/07/07 02:42:00]

I don't like this being here, but the only alternative I see is to make everyone
who wants to do an MSan build to also have to set v8_target_cpu explicitly in
their args file, and I don't know if that's acceptable.

If we do want to make that change, I would prefer to land this as-is for now to
unblock progress on monochrome and the lkgr builders, and then deal with that in
a follow-up CL.

[Michael Achenbach, 2016/07/07 07:24:22]

I'd find it ok if developers need to specify v8_target_cpu explicitly. In the
infrastructure we talk about max 5 bots, where we'd need to add this config.

Furthermore: It is not incorrect to run x64 v8 with msan. Using the simulator
just finds a few more bugs in v8, but is also slower. I think this condition
below is pure convenience.

[Dirk Pranke, 2016/07/07 17:28:12]

Ah, I thought that x64 v8 msan didn't work at all. If it does work, then I agree
that we should probably just get rid of this and update the bots (and devs).

I'd still like to do that in a follow-on CL, to make things easier, though. 
Let's see what brettw@ says.

[brettw, 2016/07/07 17:56:21]

I have a negative reaction to this since I spent a lot of effort trying to get
this exact variable (with the other *san ones) out of this file.

It this is just to support a specific config with a few bots, I think we should
just specify the toolchain we want on those bots.

[Michael Achenbach, 2016/07/08 06:57:11]

I'm not totally sure if x64 msan would work well with x64 v8. Maybe double check
with kcc and eugenis. But even if it wouldn't, we could still require the extra
setting of v8_target_cpu as gn arg and maybe assert the correct configuration
deeper in the build configs (e.g. in v8).

+
   # DON'T ADD MORE FLAGS HERE. Read the comment above.
 }
 
+if (v8_target_cpu == "") {

[brettw, 2016/07/07 17:56:21]

I really think we should avoid having V8 stuff in this file. WHat other options
are there?

+  if (is_msan) {
+    # Running the V8-generated code on an ARM simulator is a powerful hack that
+    # allows the tool to see the memory accesses from JITted code. Without this
+    # flag, JS code causes false positive reports from MSan.
+    v8_target_cpu = "arm64"
+  } else {
+    v8_target_cpu = target_cpu
+  }
+}
+
+declare_args() {

[Michael Achenbach, 2016/07/07 07:24:22]

Curiosity: Is there a semantic difference in having a second declare_args block?
Is this purely for depending on v8_target_cpu from the other block?

[Dirk Pranke, 2016/07/07 17:28:12]

Generically speaking, yes. See crbug.com/542846. This particular usage
is a little different than what you see in that bug, though.
This is done this way so that we can both get the right default value for 
v8_current_cpu if v8_target_cpu isn't set by the user, and also get the 
right value if it is set.

+  # This argument is declared here so that it can be overridden in
+  # toolchains. It should never be explicitly set by the user.

[michaelbai, 2016/07/07 18:41:26]

The v8_current_cpu will show up in the result of "gn args <out_dir> --list",
right? beside check the comment, is there other way to tell that this variable
actually shouldn't be set as args.

+  v8_current_cpu = v8_target_cpu
+}
+
 assert(!(is_debug && is_official_build), "Can't do official debug builds")
 
 # ==============================================================================
 # TOOLCHAIN SETUP
 # ==============================================================================
 #
 # Here we set the default toolchain, as well as the variable host_toolchain
 # which will identify the toolchain corresponding to the local system when
 # doing cross-compiles. When not cross-compiling, this will be the same as the
 # default toolchain.
@@ skipping 50 matching lines @@
   assert(host_os == "linux",
          "ChromeOS builds are only supported on Linux hosts.")
   if (is_clang) {
     _default_toolchain = "//build/toolchain/cros:clang_target"
   } else {
     _default_toolchain = "//build/toolchain/cros:target"
   }
 } else if (target_os == "ios") {
   _default_toolchain = "//build/toolchain/mac:ios_clang_$target_cpu"
 } else if (target_os == "linux") {
+  if (v8_current_cpu != current_cpu) {
+    _target_cpus = "${target_cpu}_v8_${v8_current_cpu}"
+  } else {
+    _target_cpus = target_cpu
+  }
   if (is_clang) {
-    _default_toolchain = "//build/toolchain/linux:clang_$target_cpu"
+    _default_toolchain = "//build/toolchain/linux:clang_${_target_cpus}"
   } else {
-    _default_toolchain = "//build/toolchain/linux:$target_cpu"
+    _default_toolchain = "//build/toolchain/linux:${_target_cpus}"
   }
 } else if (target_os == "mac") {
   assert(host_os == "mac", "Mac cross-compiles are unsupported.")
   _default_toolchain = host_toolchain
 } else if (target_os == "win") {
   # On Windows we use the same toolchain for host and target by default.
   assert(target_os == host_os, "Win cross-compiles only work on win hosts.")
   if (is_clang) {
     _default_toolchain = "//build/toolchain/win:clang_$target_cpu"
   } else {
@@ skipping 426 matching lines @@
     forward_variables_from(invoker, "*", [ "visibility" ])
 
     # All shared libraries must have the sanitizer deps to properly link in
     # asan mode (this target will be empty in other cases).
     if (!defined(deps)) {
       deps = []
     }
     deps += [ "//build/config/sanitizers:deps" ]
   }
 }