[PATCH] Implement ADRP CSE for global symbols

[Quentin Colombet]

Hi Jiangning,

I confirm that the changes you did fix the “regression” on ARM64, however I do not think this is the way to go.
Indeed, what fixes the problem, as you know, is the fact that we do not set any alignment information for internal globals. Because of this lack of information, the ARM64 backend does not try to fold the ADDlow into the memory operation since it may not be correct.

That said, if we can set some reasonable alignment, we should do it, and not backed this off because of ARM64 current limitation. Nevertheless, we need to be careful not to introduce any regression here because so far the global merge pass was producing unaligned globals.

Now, regarding a reasonable alignment, I do not think the current setting make sense.
Setting the alignment to the size of the struct looks wrong to me (think a struct with 20 times the same type). I guess this information should be set using the data layout on MergedTy. After all, all the types are currently compliant with the ABI (otherwise, we excluded the related globals).

I’ve also a couple of minor comments, see the inline comments.

I am looking into making the folding smarter to avoid the redundant ADRP.

Thanks,
-Quentin

================
Comment at: lib/Transforms/Scalar/GlobalMerge.cpp:167
@@ -161,1 +166,3 @@
+
+    bool IsExternal = Globals[i]->hasExternalLinkage();
     for (j = i; j != e; ++j) {
----------------
Should be hoisted outside of the loop, since we assert this does not change within the loop.

================
Comment at: lib/Transforms/Scalar/GlobalMerge.cpp:188
@@ +187,3 @@
+    // the symbol after merging.
+    GlobalValue::LinkageTypes Linkage = IsExternal ?
+                                          GlobalValue::ExternalLinkage :
----------------
Ditto.

================
Comment at: lib/Transforms/Scalar/GlobalMerge.cpp:203
@@ +202,3 @@
+    if (EnableGlobalMergeOnExternal && IsExternal) {
+      // If the alignment is not a power of 2, round up to the next power of 2.
+      uint64_t Align = MergedSize;
----------------
This is the alignment thing I was talking about.

http://reviews.llvm.org/D3432