[PATCH] D51377: [NFC] Make getPreferredAlignment honor section markings.

[Eli Friedman via Phabricator via llvm-commits]

efriedma created this revision.
efriedma added reviewers: hfinkel, davide, echristo.

This should more accurately reflect what the AsmPrinter will actually do.

This is NFC, as far as I can tell; all the places that might be affected already have an extra check to avoid using the result of getPreferredAlignment in this situation.


Repository:
  rL LLVM

https://reviews.llvm.org/D51377

Files:
  lib/CodeGen/GlobalMerge.cpp
  lib/IR/DataLayout.cpp


Index: lib/IR/DataLayout.cpp
===================================================================
--- lib/IR/DataLayout.cpp
+++ lib/IR/DataLayout.cpp
@@ -808,15 +808,29 @@
 /// global.  This includes an explicitly requested alignment (if the global
 /// has one).
 unsigned DataLayout::getPreferredAlignment(const GlobalVariable *GV) const {
+  unsigned GVAlignment = GV->getAlignment();
+  // If a section is specified, always precisely honor explicit alignment,
+  // so we don't insert padding into a section we don't control.
+  if (GVAlignment && GV->hasSection())
+    return GVAlignment;
+
+  // If no explicit alignment is specified, compute the alignment based on
+  // the IR type. If an alignment is specified, increase it to match the ABI
+  // alignment of the IR type.
+  //
+  // FIXME: Not sure it makes sense to use the alignment of the type if
+  // there's already an explicit alignment specification.
   Type *ElemType = GV->getValueType();
   unsigned Alignment = getPrefTypeAlignment(ElemType);
-  unsigned GVAlignment = GV->getAlignment();
   if (GVAlignment >= Alignment) {
     Alignment = GVAlignment;
   } else if (GVAlignment != 0) {
     Alignment = std::max(GVAlignment, getABITypeAlignment(ElemType));
   }
 
+  // If no explicit alignment is specified, and the global is large, increase
+  // the alignment to 16.
+  // FIXME: Why 16, specifically?
   if (GV->hasInitializer() && GVAlignment == 0) {
     if (Alignment < 16) {
       // If the global is not external, see if it is large.  If so, give it a
Index: lib/CodeGen/GlobalMerge.cpp
===================================================================
--- lib/CodeGen/GlobalMerge.cpp
+++ lib/CodeGen/GlobalMerge.cpp
@@ -462,17 +462,8 @@
     for (j = i; j != -1; j = GlobalSet.find_next(j)) {
       Type *Ty = Globals[j]->getValueType();
 
-      // Make sure we use the same alignment AsmPrinter would use. There
-      // currently isn't any helper to compute that, so we compute it
-      // explicitly here.
-      //
-      // getPreferredAlignment will sometimes return an alignment higher
-      // than the explicitly specified alignment; we must ignore that
-      // if the section is explicitly specified, to avoid inserting extra
-      // padding into that section.
+      // Make sure we use the same alignment AsmPrinter would use.
       unsigned Align = DL.getPreferredAlignment(Globals[j]);
-      if (Globals[j]->hasSection() && Globals[j]->getAlignment())
-        Align = Globals[j]->getAlignment();
       unsigned Padding = alignTo(MergedSize, Align) - MergedSize;
       MergedSize += Padding;
       MergedSize += DL.getTypeAllocSize(Ty);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D51377.162934.patch
Type: text/x-patch
Size: 2653 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180828/c109bc45/attachment.bin>