'[PATCH] Pass -mglobal-merge as a module flag metadata.'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    [PATCH] Pass -mglobal-merge as a module flag metadata.
From:       Ahmed Bougacha <ahmed.bougacha () gmail ! com>
Date:       2015-02-28 2:52:33
Message-ID: differential-rev-PHID-DREV-cgukpnp3jsxsdohkekod-req () reviews ! llvm ! org
[Download RAW message or body]

Hi dexonsmith,

Per title, we used to pass -mno-global-merge when specified, and enable Glo=
balMerge always (leaving it to LLVM to decide whether to actually enable it=
: it does, on ARM and AArch64, for -O1 and above).

This addresses two problems (I'm fine with splitting the patch if desired):
- with LTO, passing -mno-global-merge didn't do anything, since LTO implies=
 -O3 from the backend's standpoint.
- GlobalMerge was enabled by -O1.  Instead, on AArch64, enable it at -O3, o=
r when -mglobal-merge is specified. (the goal is to have it predicated by -=
O3 on ARM as well, but I still need to measure before doing that.)

Now, -mglobal-merge and -mno-global-merge are both explicitly passed by the=
 driver.
They are then passed to the backend using module flag metadata (bikesheddin=
g time: name is prose, and value is string.  We can change both, and AFAICT=
 there's no similar backend flag to be consistent with, yet.)

Thanks!
-Ahmed

http://reviews.llvm.org/D7968

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  lib/Driver/Tools.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/global-merge.c
  test/Driver/mglobal-merge.c
  test/Driver/mno-global-merge.c

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/

["D7968.20925.patch" (text/x-patch)]

Index: include/clang/Driver/Options.td
===================================================================

--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1126,7 +1126,7 @@
 def mfpmath_EQ : Joined<["-"], "mfpmath=">, Group<m_Group>;
 def mfpu_EQ : Joined<["-"], "mfpu=">, Group<m_Group>;
 def mhwdiv_EQ : Joined<["-"], "mhwdiv=">, Group<m_Group>;
-def mglobal_merge : Flag<["-"], "mglobal-merge">, Group<m_Group>;
+def mglobal_merge : Flag<["-"], "mglobal-merge">, Group<m_Group>, Flags<[CC1Option]>;
 def mhard_float : Flag<["-"], "mhard-float">, Group<m_Group>;
 def miphoneos_version_min_EQ : Joined<["-"], "miphoneos-version-min=">, Group<m_Group>;
 def mios_version_min_EQ : Joined<["-"], "mios-version-min=">,
Index: include/clang/Frontend/CodeGenOptions.def
===================================================================
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -77,7 +77,7 @@
 CODEGENOPT(FatalWarnings     , 1, 0) ///< Set when -Wa,--fatal-warnings is
                                      ///< enabled.
 CODEGENOPT(EnableSegmentedStacks , 1, 0) ///< Set when -fsplit-stack is enabled.
-CODEGENOPT(NoGlobalMerge     , 1, 0) ///< Set when -mno-global-merge is enabled.
+CODEGENOPT(EnableGlobalMerge , 1, 0) ///< Set when -mglobal-merge is enabled.
 CODEGENOPT(NoImplicitFloat   , 1, 0) ///< Set when -mno-implicit-float is enabled.
 CODEGENOPT(NoInfsFPMath      , 1, 0) ///< Assume FP arguments, results not +-Inf.
 CODEGENOPT(NoSignedZeros     , 1, 0) ///< Allow ignoring the signedness of FP zero
Index: lib/CodeGen/BackendUtil.cpp
===================================================================
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -433,8 +433,6 @@
     BackendArgs.push_back("-time-passes");
   for (unsigned i = 0, e = CodeGenOpts.BackendOptions.size(); i != e; ++i)
     BackendArgs.push_back(CodeGenOpts.BackendOptions[i].c_str());
-  if (CodeGenOpts.NoGlobalMerge)
-    BackendArgs.push_back("-enable-global-merge=false");
   BackendArgs.push_back(nullptr);
   llvm::cl::ParseCommandLineOptions(BackendArgs.size() - 1,
                                     BackendArgs.data());
Index: lib/CodeGen/CodeGenModule.cpp
===================================================================
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -418,6 +418,8 @@
   EmitVersionIdentMetadata();
 
   EmitTargetMetadata();
+
+  EmitBackendOptionsMetadata();
 }
 
 void CodeGenModule::UpdateCompletedType(const TagDecl *TD) {
@@ -3609,6 +3611,12 @@
   }
 }
 
+void CodeGenModule::EmitBackendOptionsMetadata() {
+  if (getCodeGenOpts().EnableGlobalMerge)
+    getModule().addModuleFlag(llvm::Module::Error, "Enable Global Merge",
+                              llvm::MDString::get("true"));
+}
+
 void CodeGenModule::EmitCoverageFile() {
   if (!getCodeGenOpts().CoverageFile.empty()) {
     if (llvm::NamedMDNode *CUNode = TheModule.getNamedMetadata("llvm.dbg.cu")) {
Index: lib/CodeGen/CodeGenModule.h
===================================================================
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -1199,6 +1199,10 @@
   /// Emits target specific Metadata for global declarations.
   void EmitTargetMetadata();
 
+  /// Emit the module flag metadata used to pass options controlling the
+  /// the backend to LLVM.
+  void EmitBackendOptionsMetadata();
+
   /// Emit the llvm.gcov metadata used to tell LLVM where to emit the .gcno and
   /// .gcda files in a way that persists in .bc files.
   void EmitCoverageFile();
Index: lib/Driver/Tools.cpp
===================================================================
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -860,12 +860,14 @@
     }
   }
 
-  // Setting -mno-global-merge disables the codegen global merge pass. Setting 
-  // -mglobal-merge has no effect as the pass is enabled by default.
+  // Forward the -mglobal-merge option, for explicit control over the
+  // global-merge pass.
   if (Arg *A = Args.getLastArg(options::OPT_mglobal_merge,
                                options::OPT_mno_global_merge)) {
     if (A->getOption().matches(options::OPT_mno_global_merge))
       CmdArgs.push_back("-mno-global-merge");
+    else
+      CmdArgs.push_back("-mglobal-merge");
   }
 
   if (!Args.hasFlag(options::OPT_mimplicit_float,
@@ -956,12 +958,14 @@
     CmdArgs.push_back("-aarch64-fix-cortex-a53-835769=1");
   }
 
-  // Setting -mno-global-merge disables the codegen global merge pass. Setting
-  // -mglobal-merge has no effect as the pass is enabled by default.
+  // Forward the -mglobal-merge option, for explicit control over the
+  // global-merge pass.
   if (Arg *A = Args.getLastArg(options::OPT_mglobal_merge,
                                options::OPT_mno_global_merge)) {
     if (A->getOption().matches(options::OPT_mno_global_merge))
       CmdArgs.push_back("-mno-global-merge");
+    else
+      CmdArgs.push_back("-mglobal-merge");
   }
 
   if (Args.hasArg(options::OPT_ffixed_x18)) {
Index: lib/Frontend/CompilerInvocation.cpp
===================================================================
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -309,6 +309,16 @@
   return "default";
 }
 
+static bool shouldEnableGlobalMergeForTarget(const llvm::Triple &T,
+                                             const unsigned OptLevel) {
+  llvm::Triple::ArchType Arch = T.getArch();
+  return (OptLevel > 0 &&
+          (Arch == llvm::Triple::arm || Arch == llvm::Triple::armeb ||
+           Arch == llvm::Triple::thumb || Arch == llvm::Triple::thumbeb)) ||
+         (OptLevel > 2 &&
+          (Arch == llvm::Triple::aarch64 || Arch == llvm::Triple::aarch64_be));
+}
+
 /// \brief Create a new Regex instance out of the string value in \p RpassArg.
 /// It returns a pointer to the newly generated Regex instance.
 static std::shared_ptr<llvm::Regex>
@@ -445,7 +455,12 @@
   Opts.NoZeroInitializedInBSS = Args.hasArg(OPT_mno_zero_initialized_in_bss);
   Opts.BackendOptions = Args.getAllArgValues(OPT_backend_option);
   Opts.NumRegisterParameters = getLastArgIntValue(Args, OPT_mregparm, 0, Diags);
-  Opts.NoGlobalMerge = Args.hasArg(OPT_mno_global_merge);
+
+  Opts.EnableGlobalMerge = Args.hasFlag(
+      OPT_mglobal_merge, OPT_mno_global_merge,
+      shouldEnableGlobalMergeForTarget(llvm::Triple(TargetOpts.Triple),
+                                       Opts.OptimizationLevel));
+
   Opts.NoExecStack = Args.hasArg(OPT_mno_exec_stack);
   Opts.FatalWarnings = Args.hasArg(OPT_massembler_fatal_warnings);
   Opts.EnableSegmentedStacks = Args.hasArg(OPT_split_stacks);
Index: test/CodeGen/global-merge.c
===================================================================
--- /dev/null
+++ test/CodeGen/global-merge.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -emit-llvm -mglobal-merge %s -o - | FileCheck %s -check-prefix=CHECK-GM
+// RUN: %clang_cc1 -emit-llvm -mno-global-merge %s -o - | FileCheck %s -check-prefix=CHECK-NGM
+
+// RUN: %clang_cc1 -emit-llvm -triple arm64-- -O1 %s -o - | FileCheck %s -check-prefix=CHECK-NGM
+// RUN: %clang_cc1 -emit-llvm -triple arm64-- -O3 %s -o - | FileCheck %s -check-prefix=CHECK-GM
+
+// RUN: %clang_cc1 -emit-llvm -triple arm-- -O1 %s -o - | FileCheck %s -check-prefix=CHECK-GM
+// RUN: %clang_cc1 -emit-llvm -triple arm-- -O3 %s -o - | FileCheck %s -check-prefix=CHECK-GM
+
+// CHECK-GM: !llvm.module.flags = !{{{.*}}}
+// CHECK-GM: !{{[0-9]+}} = !{i32 1, !"Enable Global Merge", !"true"}
+// CHECK-NGM: !llvm.module.flags = !{{{.*}}}
+// CHECK-NGM: !{{[0-9]+}} = !{i32 1, !"Enable Global Merge", !"false"}
Index: test/Driver/mglobal-merge.c
===================================================================
--- test/Driver/mglobal-merge.c
+++ test/Driver/mglobal-merge.c
@@ -16,5 +16,5 @@
 // RUN:   -mglobal-merge -### -fsyntax-only %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-GM < %t %s
 
-// CHECK-GM-NOT: "-mglobal-merge"
+// CHECK-GM: "-mglobal-merge"
 


_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits


[prev in list] [next in list] [prev in thread] [next in thread] 
