[PATCH] D10965: [GlobalMerge] Allow targets to enable merging of extern variables, NFC.

Ahmed Bougacha ahmed.bougacha at gmail.com
Mon Jul 6 12:11:20 PDT 2015


ab accepted this revision.

This revision is now accepted and ready to land.

A couple nits, but this sounds reasonable.  Obviously, please wait for http://reviews.llvm.org/D10966 to be approved.


================
Comment at: lib/CodeGen/GlobalMerge.cpp:133
@@ +132,3 @@
+    /// Whether we should merge global variables that have external linkage.
+    bool GlobalMergeOnExternal;
+
----------------
"GlobalMerge" isn't a verb yet ;)  How about "[Should]MergeExternalGlobals" or something similar?

================
Comment at: lib/CodeGen/GlobalMerge.cpp:610-614
@@ +609,7 @@
+                                  bool MergeExternalByDefault) {
+  bool MergeExternal;
+  if (EnableGlobalMergeOnExternal == cl::BOU_UNSET)
+    MergeExternal = MergeExternalByDefault;
+  else
+    MergeExternal = (EnableGlobalMergeOnExternal == cl::BOU_TRUE);
+  return new GlobalMerge(TM, Offset, OnlyOptimizeForSize, MergeExternal);
----------------
How about:

  bool MergeExternal = MergeExternalByDefault
  if (EnableGlobalMergeOnExternal != cl::BOU_UNSET)
    MergeExternal = (EnableGlobalMergeOnExternal == cl::BOU_TRUE);

(or the even terser ternary version).


Repository:
  rL LLVM

http://reviews.llvm.org/D10965







More information about the llvm-commits mailing list