Remove more MSVC compat hacks

Including PGO / WPO flags on Windows, which as far as I know aren't used by anyone. Bug: 1053958 Change-Id: I9479a10f921ca066ec2b702867ba687cdcb862ef Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2062880 Reviewed-by: Ken Rockot <rockot@google.com> Reviewed-by: vmpstr <vmpstr@chromium.org> Reviewed-by: Hans Wennborg <hans@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Commit-Queue: Nico Weber <thakis@chromium.org> Cr-Commit-Position: refs/heads/master@{#742850}
Denger-Network · Feb 19, 2020 · 7682724 · 7682724
1 parent 323bf70
commit 7682724
Show file tree

Hide file tree

Showing 15 changed files with 9 additions and 156 deletions.
diff --git a/build/config/compiler/BUILD.gn b/build/config/compiler/BUILD.gn
@@ -1893,32 +1893,14 @@ if (is_win) {
   common_optimize_on_ldflags = []
 
   # /OPT:ICF is not desirable in Debug builds, since code-folding can result in
-  # misleading symbols in stack traces. It is also incompatible with
-  # incremental linking, which we enable for both Debug and component builds.
+  # misleading symbols in stack traces.
   if (!is_debug && !is_component_build) {
     common_optimize_on_ldflags += [ "/OPT:ICF" ]  # Redundant COMDAT folding.
   }
 
   if (is_official_build) {
     common_optimize_on_ldflags += [ "/OPT:REF" ]  # Remove unreferenced data.
-
-    # TODO(thakis): Remove is_clang here, https://crbug.com/598772
-    if (!use_lld && !is_clang) {
-      common_optimize_on_ldflags += [ "/cgthreads:" + max_jobs_per_link ]
-      if (use_incremental_wpo) {
-        # Incremental Link-time code generation.
-        common_optimize_on_ldflags += [ "/LTCG:INCREMENTAL" ]
-      } else {
-        common_optimize_on_ldflags += [ "/LTCG" ]  # Link-time code generation.
-      }
-      if (full_wpo_on_official) {
-        if (use_incremental_wpo) {
-          arflags = [ "/LTCG:INCREMENTAL" ]
-        } else {
-          arflags = [ "/LTCG" ]
-        }
-      }
-    }
+    # TODO(thakis): Add LTO/PGO clang flags eventually, https://crbug.com/598772
   }
 } else {
   common_optimize_on_cflags = []
@@ -2001,15 +1983,13 @@ config("default_stack_frames") {
 # Default "optimization on" config.
 config("optimize") {
   if (is_win) {
-    # TODO(thakis): Remove is_clang here, https://crbug.com/598772
-    if (is_official_build && full_wpo_on_official && !is_clang) {
-      common_optimize_on_cflags += [ "/GL" ]  # Whole program optimization.
-    }
-
     # Favor size over speed, /O1 must be before the common flags. The GYP
     # build also specifies /Os and /GF but these are implied by /O1.
     cflags = [ "/O1" ] + common_optimize_on_cflags + [ "/Oi" ]
-  } else if (optimize_for_size && !is_nacl) {
+
+    # TODO(thakis): Use -flto in official builds here eventually,
+    # https://crbug.com/598772
+} else if (optimize_for_size && !is_nacl) {
     # Favor size over speed.
     # TODO(crbug.com/718650): Fix -Os in PNaCl compiler and remove the is_nacl
     # guard above.

diff --git a/build/config/compiler/compiler.gni b/build/config/compiler/compiler.gni
@@ -80,10 +80,6 @@ declare_args() {
   # can be used to convert a fastlink pdb to a normal one.
   is_win_fastlink = false
 
-  # Whether or not we should turn on incremental WPO. Only affects the VS
-  # Windows build.
-  use_incremental_wpo = false
-
   # Whether we're using a sample profile collected on an architecture different
   # than the one we're compiling for.
   #
@@ -182,16 +178,6 @@ assert(!can_unwind_with_frame_pointers || enable_frame_pointers)
 can_unwind_with_cfi_table = is_android && !is_component_build &&
                             !enable_frame_pointers && current_cpu == "arm"
 
-declare_args() {
-  # Whether or not the official builds should be built with full WPO. Enabled by
-  # default for the PGO and the x64 builds.
-  if (chrome_pgo_phase > 0) {
-    full_wpo_on_official = true
-  } else {
-    full_wpo_on_official = false
-  }
-}
-
 declare_args() {
   # Set to true to use lld, the LLVM linker.
   use_lld = is_clang && (!is_ios && !is_mac)

diff --git a/build/config/compiler/pgo/BUILD.gn b/build/config/compiler/pgo/BUILD.gn
@@ -23,23 +23,6 @@ config("pgo_instrumentation_flags") {
         # dependent library into each object file.
         ldflags += [ "-fprofile-instr-generate" ]
       }
-    } else if (is_win) {
-      ldflags = [
-        # In MSVC, we must use /LTCG when using PGO.
-        "/LTCG",
-
-        # Make sure that enough memory gets allocated for the PGO profiling
-        # buffers and also cap this memory. Usually a PGI instrumented build
-        # of chrome_child.dll requires ~55MB of memory for storing its counter
-        # etc, normally the linker should automatically choose an appropriate
-        # amount of memory but it doesn't always do a good estimate and
-        # sometime allocates too little or too much (and so the instrumented
-        # image fails to start). Making sure that the buffer has a size in the
-        # [128 MB, 512 MB] range should prevent this from happening.
-        "/GENPROFILE:MEMMIN=134217728",
-        "/GENPROFILE:MEMMAX=536870912",
-        "/PogoSafeMode",
-      ]
     }
   }
 }
@@ -65,12 +48,6 @@ config("pgo_optimization_flags") {
         "-Wno-profile-instr-unprofiled",
         "-Wno-error=profile-instr-out-of-date",
       ]
-    } else if (is_win) {
-      ldflags += [
-        # In MSVC, we must use /LTCG when using PGO.
-        "/LTCG",
-        "/USEPROFILE",
-      ]
     }
   }
 }

diff --git a/build/config/compiler/pgo/pgo.gni b/build/config/compiler/pgo/pgo.gni
@@ -8,8 +8,6 @@ declare_args() {
   #     0 : Means that PGO is turned off.
   #     1 : Used during the PGI (instrumentation) phase.
   #     2 : Used during the PGO (optimization) phase.
-  #
-  # TODO(sebmarchand): Add support for the PGU (update) phase.
   chrome_pgo_phase = 0
 
   # When using chrome_pgo_phase = 2, read profile data from this path.

diff --git a/cc/BUILD.gn b/cc/BUILD.gn
@@ -748,13 +748,6 @@ cc_test("cc_unittests") {
     sources += [ "paint/skottie_transfer_cache_entry_unittest.cc" ]
   }
 
-  if (is_win) {
-    # TODO(vmpstr): Some SoftwareImageDecodeCacheTests use virtual inheritance,
-    # which MSVC doesn't like. Suppress "Foo inherits Bar via dominance"
-    # warnings for now.
-    cflags = [ "/wd4250" ]
-  }
-
   deps = [
     ":cc",
     ":test_support",

diff --git a/chrome/chrome_elf/BUILD.gn b/chrome/chrome_elf/BUILD.gn
@@ -160,13 +160,6 @@ static_library("nt_registry") {
     "nt_registry/nt_registry.cc",
     "nt_registry/nt_registry.h",
   ]
-  if (is_official_build && full_wpo_on_official == true) {
-    # This library doen't build with WPO enabled due to a MSVC compiler bug.
-    # TODO(pennymac|sebmarchand): Remove this once MS has fixed this compiler
-    # bug: https://connect.microsoft.com/VisualStudio/feedback/details/3104499
-    configs -= [ "//build/config/compiler:default_optimization" ]
-    configs += [ "//build/config/compiler:optimize_no_wpo" ]
-  }
 
   libs = [ "kernel32.lib" ]
 }

diff --git a/components/cbor/writer.cc b/components/cbor/writer.cc
@@ -134,11 +134,6 @@ bool Writer::EncodeCBOR(const Value& node,
       return true;
     }
   }
-
-  // This is needed because, otherwise, MSVC complains that not all paths return
-  // a value. We should be able to remove it once MSVC builders are gone.
-  NOTREACHED();
-  return false;
 }
 
 void Writer::StartItem(Value::Type type, uint64_t size) {

diff --git a/content/ppapi_plugin/ppapi_thread.h b/content/ppapi_plugin/ppapi_thread.h
@@ -46,12 +46,6 @@ namespace content {
 
 class PpapiBlinkPlatformImpl;
 
-#if defined(COMPILER_MSVC)
-// See explanation for other RenderViewHostImpl which is the same issue.
-#pragma warning(push)
-#pragma warning(disable: 4250)
-#endif
-
 class PpapiThread : public ChildThreadImpl,
                     public ppapi::proxy::PluginDispatcher::PluginDelegate,
                     public ppapi::proxy::PluginProxyDelegate {
@@ -187,10 +181,6 @@ class PpapiThread : public ChildThreadImpl,
   DISALLOW_IMPLICIT_CONSTRUCTORS(PpapiThread);
 };
 
-#if defined(COMPILER_MSVC)
-#pragma warning(pop)
-#endif
-
 }  // namespace content
 
 #endif  // CONTENT_PPAPI_PLUGIN_PPAPI_THREAD_H_
diff --git a/content/renderer/render_thread_impl.h b/content/renderer/render_thread_impl.h
@@ -112,12 +112,6 @@ class VariationsRenderThreadObserver;
 class StreamTextureFactory;
 #endif
 
-#if defined(COMPILER_MSVC)
-// See explanation for other RenderViewHostImpl which is the same issue.
-#pragma warning(push)
-#pragma warning(disable: 4250)
-#endif
-
 // The RenderThreadImpl class represents the main thread, where RenderView
 // instances live.  The RenderThread supports an API that is used by its
 // consumer to talk indirectly to the RenderViews and supporting objects.
@@ -687,10 +681,6 @@ class CONTENT_EXPORT RenderThreadImpl
   DISALLOW_COPY_AND_ASSIGN(RenderThreadImpl);
 };
 
-#if defined(COMPILER_MSVC)
-#pragma warning(pop)
-#endif
-
 }  // namespace content
 
 #endif  // CONTENT_RENDERER_RENDER_THREAD_IMPL_H_
diff --git a/content/test/test_render_view_host.h b/content/test/test_render_view_host.h
@@ -138,12 +138,6 @@ class TestRenderWidgetHostView : public RenderWidgetHostViewBase,
 #endif
 };
 
-#if defined(COMPILER_MSVC)
-// See comment for same warning on RenderViewHostImpl.
-#pragma warning(push)
-#pragma warning(disable: 4250)
-#endif
-
 // TestRenderViewHost ----------------------------------------------------------
 
 // TODO(brettw) this should use a TestWebContents which should be generalized
@@ -264,10 +258,6 @@ class TestRenderViewHost
   DISALLOW_COPY_AND_ASSIGN(TestRenderViewHost);
 };
 
-#if defined(COMPILER_MSVC)
-#pragma warning(pop)
-#endif
-
 // Adds methods to get straight at the impl classes.
 class RenderViewHostImplTestHarness : public RenderViewHostTestHarness {
  public:

diff --git a/content/utility/utility_thread_impl.h b/content/utility/utility_thread_impl.h
@@ -17,12 +17,6 @@
 namespace content {
 class UtilityServiceFactory;
 
-#if defined(COMPILER_MSVC)
-// See explanation for other RenderViewHostImpl which is the same issue.
-#pragma warning(push)
-#pragma warning(disable: 4250)
-#endif
-
 // This class represents the background thread where the utility task runs.
 class UtilityThreadImpl : public UtilityThread,
                           public ChildThreadImpl {
@@ -59,10 +53,6 @@ class UtilityThreadImpl : public UtilityThread,
   DISALLOW_COPY_AND_ASSIGN(UtilityThreadImpl);
 };
 
-#if defined(COMPILER_MSVC)
-#pragma warning(pop)
-#endif
-
 }  // namespace content
 
 #endif  // CONTENT_UTILITY_UTILITY_THREAD_IMPL_H_
diff --git a/ipc/ipc_message_macros.h b/ipc/ipc_message_macros.h
@@ -270,16 +270,8 @@
   template class EXPORT_TEMPLATE_DEFINE(IPC_MESSAGE_EXPORT) \
       IPC::MessageT<msg_name##_Meta>;
 
-// MSVC has an intentionally non-compliant "feature" that results in LNK2005
-// ("symbol already defined") errors if we provide an out-of-line definition
-// for kKind.  Microsoft's official response is to test for _MSC_EXTENSIONS:
-// https://connect.microsoft.com/VisualStudio/feedback/details/786583/
-#if defined(_MSC_EXTENSIONS)
-#define IPC_MESSAGE_DEFINE_KIND(msg_name)
-#else
 #define IPC_MESSAGE_DEFINE_KIND(msg_name) \
   const IPC::MessageKind msg_name##_Meta::kKind;
-#endif
 
 #elif defined(IPC_MESSAGE_MACROS_LOG_ENABLED)
 

diff --git a/ipc/ipc_message_templates.h b/ipc/ipc_message_templates.h
@@ -92,15 +92,8 @@ struct Routing {
 // We want to restrict MessageT's constructors so that a routing_id is always
 // provided for ROUTED messages and never provided for CONTROL messages, so
 // use the SFINAE technique from N4387's "Implementation Hint" section.
-#if defined(COMPILER_MSVC)
-// MSVC 2013 doesn't support default arguments for template member functions
-// of templated classes, so there we have to rely on the DCHECKs instead.
-// TODO(mdempsky): Reevaluate once MSVC 2015.
-#define IPC_MESSAGET_SFINAE(x)
-#else
 #define IPC_MESSAGET_SFINAE(x) \
   template <bool X = (x), typename std::enable_if<X, bool>::type = false>
-#endif
 
 // MessageT is the common template used for all user-defined message types.
 // It's intended to be used via the macros defined in ipc_message_macros.h.

diff --git a/third_party/yasm/BUILD.gn b/third_party/yasm/BUILD.gn
@@ -335,14 +335,8 @@ if (current_toolchain == host_toolchain) {
       ":yasm_warnings",
     ]
 
-    # Disable WPO for yasm: crbug.com/604808
-    if (is_official_build && full_wpo_on_official) {
-      configs -= [ "//build/config/compiler:default_optimization" ]
-      configs += [ "//build/config/compiler:optimize_no_wpo" ]
-    } else {
-      configs -= configs_to_delete
-      configs += configs_to_add
-    }
+    configs -= configs_to_delete
+    configs += configs_to_add
 
     # Yasm generates a bunch of .c files which its source file #include. These
     # are placed in |yasm_gen_include_dir|.

diff --git a/ui/base/class_property.h b/ui/base/class_property.h
@@ -140,9 +140,7 @@ class UI_BASE_EXPORT PropertyHandler {
 namespace {
 
 // No single new-style cast works for every conversion to/from int64_t, so we
-// need this helper class. A third specialization is needed for bool because
-// MSVC warning C4800 (forcing value to bool) is not suppressed by an explicit
-// cast (!).
+// need this helper class.
 template<typename T>
 class ClassPropertyCaster {
  public:
@@ -155,12 +153,6 @@ class ClassPropertyCaster<T*> {
   static int64_t ToInt64(T* x) { return reinterpret_cast<int64_t>(x); }
   static T* FromInt64(int64_t x) { return reinterpret_cast<T*>(x); }
 };
-template<>
-class ClassPropertyCaster<bool> {
- public:
-  static int64_t ToInt64(bool x) { return static_cast<int64_t>(x); }
-  static bool FromInt64(int64_t x) { return x != 0; }
-};
 template <>
 class ClassPropertyCaster<base::TimeDelta> {
  public: