[PATCH] D10966: [ARM] Make GlobalMerge merge extern globals by default

[Ahmed Bougacha]

ab requested changes to this revision.

This revision now requires changes to proceed.

So, if this is positive performance-wise, I'm fine with it, but I don't think you can enable it on all platforms (at least not without testing).

For instance, this won't work on Mach-O, most importantly because aliases are just regular symbols, and symbols are (usually) assumed atomic;  __MergedGlobals wouldn't be in this case.

On a related note, on targets where aliases aren't an issue, I think we should use them all the time, even when merging internal globals.  It lets us

- get rid of cryptic _MergedGlobals symbols
- use the source symbols instead
- simplify some DebugInfo weirdness (you won't need a 'plus' expression if you can just reference a symbol).

That's where I discovered the Mach-O symbol alias issue!


================
Comment at: lib/Target/ARM/ARMTargetMachine.cpp:354
@@ -353,1 +353,3 @@
+    // generally either beneficial or harmless.
+    addPass(createGlobalMergePass(TM, 127, OnlyOptimizeForSize, true));
   }
----------------
Even with the comment, can you add:

  /*MergeExternalByDefault=*/true


Repository:
  rL LLVM

http://reviews.llvm.org/D10966