[builtins] Emit builtins as inline assembly on windows clang builds

We recently changed embedded builtins to be emitted as raw assembly
files during the build process in order to support MSVC (which doesn't
support inline assembly on x64). Ninja uses ml.exe / ml64.exe as the
assembler on all Windows builds (msvc & clang); these unfortunately
don't support large data streams well and can take over 5 minutes for
embedded.S.

With this CL we work around this by going back to inlined assembly for
clang Windows builds.

Bug: v8:6666, v8:8475
Change-Id: I33beb3f5a1df07de3299df0fc2be4e8983701db0
Reviewed-on: https://chromium-review.googlesource.com/c/1344114
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Sergiy Belozorov <sergiyb@chromium.org>
Reviewed-by: Michael Stanton <mvstanton@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57726}

BUILD.gn

@@ -24,6 +24,9 @@ import("snapshot_toolchain.gni")
 is_target_simulator = (target_cpu != v8_target_cpu && !v8_multi_arch_build) ||
                       (current_cpu != v8_current_cpu && v8_multi_arch_build)
 
+# For faster Windows builds. See https://crbug.com/v8/8475.
+emit_builtins_as_inline_asm = is_win && is_clang
+
 declare_args() {
   # Print to stdout on Android.
   v8_android_log_stdout = false
@@ -398,10 +401,7 @@ config("features") {
     defines += [ "V8_CHECK_MICROTASKS_SCOPES_CONSISTENCY" ]
   }
   if (v8_enable_embedded_builtins) {
-    defines += [
-      "V8_EMBEDDED_BUILTINS",
-      "V8_EMBEDDED_BYTECODE_HANDLERS",
-    ]
+    defines += [ "V8_EMBEDDED_BUILTINS" ]
   }
   if (v8_enable_jitless_mode) {
     defines += [ "V8_JITLESS_MODE" ]
@@ -680,6 +680,39 @@ config("v8_gcov_coverage_ldflags") {
 # Actions
 #
 
+# Only for Windows clang builds. Converts the embedded.S file produced by
+# mksnapshot into an embedded.cc file with corresponding inline assembly.
+template("asm_to_inline_asm") {
+  name = target_name
+  if (name == "default") {
+    suffix = ""
+  } else {
+    suffix = "_$name"
+  }
+
+  action("asm_to_inline_asm_" + name) {
+    visibility = [ ":*" ]  # Only targets in this file can depend on this.
+
+    assert(emit_builtins_as_inline_asm)
+
+    script = "tools/snapshot/asm_to_inline_asm.py"
+    deps = [
+      ":run_mksnapshot_" + name,
+    ]
+    sources = [
+      "$target_gen_dir/embedded${suffix}.S",
+    ]
+    outputs = [
+      "$target_gen_dir/embedded${suffix}.cc",
+    ]
+    args = invoker.args
+    args += [
+      rebase_path("$target_gen_dir/embedded${suffix}.S", root_build_dir),
+      rebase_path("$target_gen_dir/embedded${suffix}.cc", root_build_dir),
+    ]
+  }
+}
+
 action("js2c") {
   visibility = [ ":*" ]  # Only targets in this file can depend on this.
 
@@ -1133,6 +1166,11 @@ if (v8_use_snapshot) {
       embedded_variant = "Default"
     }
   }
+  if (emit_builtins_as_inline_asm) {
+    asm_to_inline_asm("default") {
+      args = []
+    }
+  }
   if (v8_use_multi_snapshots) {
     run_mksnapshot("trusted") {
       args = [ "--no-untrusted-code-mitigations" ]
@@ -1140,6 +1178,11 @@ if (v8_use_snapshot) {
         embedded_variant = "Trusted"
       }
     }
+    if (emit_builtins_as_inline_asm) {
+      asm_to_inline_asm("trusted") {
+        args = []
+      }
+    }
   }
 }
 
@@ -1267,7 +1310,10 @@ if (v8_use_snapshot && !v8_use_external_startup_data) {
       "src/setup-isolate-deserialize.cc",
     ]
 
-    if (v8_enable_embedded_builtins) {
+    if (v8_enable_embedded_builtins && emit_builtins_as_inline_asm) {
+      deps += [ ":asm_to_inline_asm_default" ]
+      sources += [ "$target_gen_dir/embedded.cc" ]
+    } else if (v8_enable_embedded_builtins) {
       sources += [ "$target_gen_dir/embedded.S" ]
     } else {
       sources += [ "src/snapshot/embedded-empty.cc" ]
@@ -1314,7 +1360,19 @@ if (v8_use_snapshot && v8_use_external_startup_data) {
     # Do not publicize any header to remove build dependency.
     public = []
 
-    if (v8_enable_embedded_builtins) {
+    if (v8_enable_embedded_builtins && emit_builtins_as_inline_asm) {
+      deps += [ ":asm_to_inline_asm_default" ]
+      sources += [ "$target_gen_dir/embedded.cc" ]
+
+      if (v8_use_multi_snapshots) {
+        deps += [ ":asm_to_inline_asm_trusted" ]
+        sources += [ "$target_gen_dir/embedded_trusted.cc" ]
+
+        if (use_jumbo_build == true) {
+          jumbo_excluded_sources = [ "$target_gen_dir/embedded_trusted.cc" ]
+        }
+      }
+    } else if (v8_enable_embedded_builtins) {
       sources += [ "$target_gen_dir/embedded.S" ]
 
       if (v8_use_multi_snapshots) {

src/snapshot/embedded-file-writer.cc

@@ -9,6 +9,23 @@
 namespace v8 {
 namespace internal {
 
+// V8_CC_MSVC is true for both MSVC and clang on windows. clang can handle
+// __asm__-style inline assembly but MSVC cannot, and thus we need a more
+// precise compiler detection that can distinguish between the two. clang on
+// windows sets both __clang__ and _MSC_VER, MSVC sets only _MSC_VER.
+#if defined(_MSC_VER) && !defined(__clang__)
+#define V8_COMPILER_IS_MSVC
+#endif
+
+// Name mangling.
+// Symbols are prefixed with an underscore on 32-bit architectures.
+#if defined(V8_OS_WIN) && !defined(V8_TARGET_ARCH_X64) && \
+    !defined(V8_TARGET_ARCH_ARM64)
+#define SYMBOL_PREFIX "_"
+#else
+#define SYMBOL_PREFIX ""
+#endif
+
 // Platform-independent bits.
 // -----------------------------------------------------------------------------
 
@@ -25,32 +42,30 @@ DataDirective PointerSizeDirective() {
 
 }  // namespace
 
-// static
-const char* PlatformDependentEmbeddedFileWriter::DirectiveAsString(
-    DataDirective directive) {
-#ifndef V8_OS_WIN
+const char* DirectiveAsString(DataDirective directive) {
+#if defined(V8_OS_WIN) && defined(V8_COMPILER_IS_MSVC)
   switch (directive) {
     case kByte:
-      return ".byte";
+      return "BYTE";
     case kLong:
-      return ".long";
+      return "DWORD";
     case kQuad:
-      return ".quad";
-    case kOcta:
-      return ".octa";
-  }
+      return "QWORD";
+    default:
       UNREACHABLE();
+  }
 #else
   switch (directive) {
     case kByte:
-      return "BYTE";
+      return ".byte";
     case kLong:
-      return "DWORD";
+      return ".long";
     case kQuad:
-      return "QWORD";
-    default:
-      UNREACHABLE();
+      return ".quad";
+    case kOcta:
+      return ".octa";
   }
+  UNREACHABLE();
 #endif
 }
 
@@ -209,11 +224,12 @@ int PlatformDependentEmbeddedFileWriter::IndentedDataDirective(
   return fprintf(fp_, "  %s ", DirectiveAsString(directive));
 }
 
-// V8_OS_WIN
+// V8_OS_WIN (MSVC)
 // -----------------------------------------------------------------------------
 
-#elif defined(V8_OS_WIN)
+#elif defined(V8_OS_WIN) && defined(V8_COMPILER_IS_MSVC)
 
+// For MSVC builds we emit assembly in MASM syntax.
 // See https://docs.microsoft.com/en-us/cpp/assembler/masm/directives-reference.
 
 void PlatformDependentEmbeddedFileWriter::SectionText() {
@@ -231,32 +247,20 @@ void PlatformDependentEmbeddedFileWriter::SectionRoData() {
 void PlatformDependentEmbeddedFileWriter::DeclareUint32(const char* name,
                                                         uint32_t value) {
   DeclareSymbolGlobal(name);
-#if defined(V8_TARGET_ARCH_X64) || defined(V8_TARGET_ARCH_ARM64)
-  fprintf(fp_, "%s %s %d\n", name, DirectiveAsString(kLong), value);
-#else
-  fprintf(fp_, "_%s %s %d\n", name, DirectiveAsString(kLong), value);
-#endif
+  fprintf(fp_, "%s%s %s %d\n", SYMBOL_PREFIX, name, DirectiveAsString(kLong),
+          value);
 }
 
 void PlatformDependentEmbeddedFileWriter::DeclarePointerToSymbol(
     const char* name, const char* target) {
   DeclareSymbolGlobal(name);
-#if defined(V8_TARGET_ARCH_X64) || defined(V8_TARGET_ARCH_ARM64)
-  fprintf(fp_, "%s %s %s\n", name, DirectiveAsString(PointerSizeDirective()),
-          target);
-#else
-  fprintf(fp_, "_%s %s _%s\n", name, DirectiveAsString(PointerSizeDirective()),
-          target);
-#endif
+  fprintf(fp_, "%s%s %s %s%s\n", SYMBOL_PREFIX, name,
+          DirectiveAsString(PointerSizeDirective()), SYMBOL_PREFIX, target);
 }
 
 void PlatformDependentEmbeddedFileWriter::DeclareSymbolGlobal(
     const char* name) {
-#if defined(V8_TARGET_ARCH_X64) || defined(V8_TARGET_ARCH_ARM64)
-  fprintf(fp_, "PUBLIC %s\n", name);
-#else
-  fprintf(fp_, "PUBLIC _%s\n", name);
-#endif
+  fprintf(fp_, "PUBLIC %s%s\n", SYMBOL_PREFIX, name);
 }
 
 void PlatformDependentEmbeddedFileWriter::AlignToCodeAlignment() {
@@ -270,28 +274,17 @@ void PlatformDependentEmbeddedFileWriter::Comment(const char* string) {
 }
 
 void PlatformDependentEmbeddedFileWriter::DeclareLabel(const char* name) {
-#if defined(V8_TARGET_ARCH_X64) || defined(V8_TARGET_ARCH_ARM64)
-  fprintf(fp_, "%s LABEL %s\n", name, DirectiveAsString(kByte));
-#else
-  fprintf(fp_, "_%s LABEL %s\n", name, DirectiveAsString(kByte));
-#endif
+  fprintf(fp_, "%s%s LABEL %s\n", SYMBOL_PREFIX, name,
+          DirectiveAsString(kByte));
 }
 
 void PlatformDependentEmbeddedFileWriter::DeclareFunctionBegin(
     const char* name) {
-#if defined(V8_TARGET_ARCH_X64) || defined(V8_TARGET_ARCH_ARM64)
-  fprintf(fp_, "%s PROC\n", name);
-#else
-  fprintf(fp_, "_%s PROC\n", name);
-#endif
+  fprintf(fp_, "%s%s PROC\n", SYMBOL_PREFIX, name);
 }
 
 void PlatformDependentEmbeddedFileWriter::DeclareFunctionEnd(const char* name) {
-#if defined(V8_TARGET_ARCH_X64) || defined(V8_TARGET_ARCH_ARM64)
-  fprintf(fp_, "%s ENDP\n", name);
-#else
-  fprintf(fp_, "_%s ENDP\n", name);
-#endif
+  fprintf(fp_, "%s%s ENDP\n", SYMBOL_PREFIX, name);
 }
 
 int PlatformDependentEmbeddedFileWriter::HexLiteral(uint64_t value) {
@@ -313,7 +306,7 @@ int PlatformDependentEmbeddedFileWriter::IndentedDataDirective(
   return fprintf(fp_, "  %s ", DirectiveAsString(directive));
 }
 
-// Everything but AIX, Windows, or OSX.
+// Everything but AIX, Windows with MSVC, or OSX.
 // -----------------------------------------------------------------------------
 
 #else
@@ -347,12 +340,13 @@ void PlatformDependentEmbeddedFileWriter::DeclarePointerToSymbol(
     const char* name, const char* target) {
   DeclareSymbolGlobal(name);
   DeclareLabel(name);
-  fprintf(fp_, "  %s %s\n", DirectiveAsString(PointerSizeDirective()), target);
+  fprintf(fp_, "  %s %s%s\n", DirectiveAsString(PointerSizeDirective()),
+          SYMBOL_PREFIX, target);
 }
 
 void PlatformDependentEmbeddedFileWriter::DeclareSymbolGlobal(
     const char* name) {
-  fprintf(fp_, ".global %s\n", name);
+  fprintf(fp_, ".global %s%s\n", SYMBOL_PREFIX, name);
 }
 
 void PlatformDependentEmbeddedFileWriter::AlignToCodeAlignment() {
@@ -364,14 +358,23 @@ void PlatformDependentEmbeddedFileWriter::Comment(const char* string) {
 }
 
 void PlatformDependentEmbeddedFileWriter::DeclareLabel(const char* name) {
-  fprintf(fp_, "%s:\n", name);
+  fprintf(fp_, "%s%s:\n", SYMBOL_PREFIX, name);
 }
 
 void PlatformDependentEmbeddedFileWriter::DeclareFunctionBegin(
     const char* name) {
   DeclareLabel(name);
 
-#if defined(V8_TARGET_ARCH_ARM) || defined(V8_TARGET_ARCH_ARM64)
+#if defined(V8_OS_WIN)
+  // The directives for inserting debugging information on Windows come
+  // from the PE (Portable Executable) and COFF (Common Object File Format)
+  // standards. Documented here:
+  // https://docs.microsoft.com/en-us/windows/desktop/debug/pe-format
+  //
+  // .scl 2 means StorageClass external.
+  // .type 32 means Type Representation Function.
+  fprintf(fp_, ".def %s%s; .scl 2; .type 32; .endef;\n", SYMBOL_PREFIX, name);
+#elif defined(V8_TARGET_ARCH_ARM) || defined(V8_TARGET_ARCH_ARM64)
   // ELF format binaries on ARM use ".type <function name>, %function"
   // to create a DWARF subprogram entry.
   fprintf(fp_, ".type %s, %%function\n", name);
@@ -400,5 +403,8 @@ int PlatformDependentEmbeddedFileWriter::IndentedDataDirective(
 
 #endif
 
+#undef SYMBOL_PREFIX
+#undef V8_COMPILER_IS_MSVC
+
 }  // namespace internal
 }  // namespace v8

src/snapshot/embedded-file-writer.h

@@ -59,8 +59,6 @@ class PlatformDependentEmbeddedFileWriter final {
  private:
   void DeclareSymbolGlobal(const char* name);
 
-  static const char* DirectiveAsString(DataDirective directive);
-
  private:
   FILE* fp_ = nullptr;
 };
@@ -76,8 +74,8 @@ class PlatformDependentEmbeddedFileWriter final {
 // The variant is usually "Default" but can be modified in multisnapshot builds.
 class EmbeddedFileWriter {
  public:
-  void SetEmbeddedFile(const char* embedded_cpp_file) {
-    embedded_cpp_path_ = embedded_cpp_file;
+  void SetEmbeddedFile(const char* embedded_src_path) {
+    embedded_src_path_ = embedded_src_path;
   }
 
   void SetEmbeddedVariant(const char* embedded_variant) {
@@ -90,9 +88,9 @@ class EmbeddedFileWriter {
 
  private:
   void MaybeWriteEmbeddedFile(const i::EmbeddedData* blob) const {
-    if (embedded_cpp_path_ == nullptr) return;
+    if (embedded_src_path_ == nullptr) return;
 
-    FILE* fp = GetFileDescriptorOrDie(embedded_cpp_path_);
+    FILE* fp = GetFileDescriptorOrDie(embedded_src_path_);
 
     PlatformDependentEmbeddedFileWriter writer;
     writer.SetFile(fp);
@@ -203,14 +201,17 @@ class EmbeddedFileWriter {
     w->FileEpilogue();
   }
 
-#ifdef V8_OS_WIN
+#if defined(_MSC_VER) && !defined(__clang__)
+#define V8_COMPILER_IS_MSVC
+#endif
+
+#ifdef V8_COMPILER_IS_MSVC
   // Windows MASM doesn't have an .octa directive, use QWORDs instead.
   // Note: MASM *really* does not like large data streams. It takes over 5
   // minutes to assemble the ~350K lines of embedded.S produced when using
   // BYTE directives in a debug build. QWORD produces roughly 120KLOC and
   // reduces assembly time to ~40 seconds. Still terrible, but much better
-  // than before.
-  // TODO(v8:8475): Use nasm or yasm instead of MASM.
+  // than before. See also: https://crbug.com/v8/8475.
 
   static constexpr DataDirective kByteChunkDirective = kQuad;
   static constexpr int kByteChunkSize = 8;
@@ -220,7 +221,7 @@ class EmbeddedFileWriter {
     const uint64_t* quad_ptr = reinterpret_cast<const uint64_t*>(data);
     return current_line_length + w->HexLiteral(*quad_ptr);
   }
-#else  // V8_OS_WIN
+#else  // V8_COMPILER_IS_MSVC
   static constexpr DataDirective kByteChunkDirective = kOcta;
   static constexpr int kByteChunkSize = 16;
 
@@ -245,7 +246,8 @@ class EmbeddedFileWriter {
     }
     return current_line_length;
   }
-#endif  // V8_OS_WIN
+#endif  // V8_COMPILER_IS_MSVC
+#undef V8_COMPILER_IS_MSVC
 
   static int WriteDirectiveOrSeparator(PlatformDependentEmbeddedFileWriter* w,
                                        int current_line_length,
@@ -303,7 +305,7 @@ class EmbeddedFileWriter {
     if (current_line_length != 0) w->Newline();
   }
 
-  const char* embedded_cpp_path_ = nullptr;
+  const char* embedded_src_path_ = nullptr;
   const char* embedded_variant_ = kDefaultEmbeddedVariant;
 };
 

tools/snapshot/asm_to_inline_asm.py

+#!/usr/bin/env python
+
+# Copyright 2018 the V8 project authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+'''
+Converts a given file in clang assembly syntax to a corresponding
+representation in inline assembly. Specifically, this is used to convert
+embedded.S to embedded.cc for Windows clang builds.
+'''
+
+import argparse
+import sys
+
+def asm_to_inl_asm(in_filename, out_filename):
+  with open(in_filename, 'r') as infile, open(out_filename, 'wb') as outfile:
+    outfile.write('__asm__(\n')
+    for line in infile:
+      outfile.write('  "%s\\n"\n' % line.rstrip())
+    outfile.write(');\n')
+  return 0
+
+if __name__ == '__main__':
+  parser = argparse.ArgumentParser(description=__doc__)
+  parser.add_argument('input', help='Name of the input assembly file')
+  parser.add_argument('output', help='Name of the target CC file')
+  args = parser.parse_args()
+  sys.exit(asm_to_inl_asm(args.input, args.output))