[MC][ELF] compute entity size for explicit sections

[nickdesaulniers]

Summary

Global variables might declare themselves to be in explicit sections.
Calculate the entity size always to prevent assembler warnings
"entity size for SHF_MERGE not specified" when sections are to be
marked merge-able.

Fixes PR31828.

[nickdesaulniers]

• prefer temporary to local

[rnk]

lgtm

[nickdesaulniers]

Thanks for the code review.

[nickdesaulniers]

This seems to be failing for other arch's: http://lab.llvm.org:8011/builders/clang-cmake-armv7-quick/builds/4325/steps/ninja%20check%201/logs/FAIL%3A%20LLVM%3A%3Asection_mergeable_size.ll

@rnk , does the CHECK need to match the full line, or is the .text appearing in the test cases problematic?

[nickdesaulniers]

@MaskRay fixed in r344206. Thanks!

[fhahn]

Looks like the test fails on MacOS: http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/54057/console

Error message:

LLVM ERROR: Global variable 'a' has an invalid section specifier '.init.rodata': mach-o section specifier requires a segment and section separated by a comma.

Not sure how to work around this, short of making it target specific?

[nickdesaulniers]

Thanks for the report, and sorry for adding a red test on OSX. The name of the section is not the important part (it could be called "foo") of the test, just that it have a custom section. The IR from the test is from real code from the Linux kernel. If the format from the error a segment and section separated by a comma is valid on Linux, then that should be fine. But I guess then this issue pops up again for Windows, or anywhere else that doesn't use ELF? So this probably should be made into an ELF only test.

I'll look into how to do that, but appreciate any tips.

[nickdesaulniers]

I should probably remove my added test file, and merge the test case into test/MC/ELF/entsize.ll.

[fhahn]

In D53056#1262126, @nickdesaulniers wrote:

I should probably remove my added test file, and merge the test case into test/MC/ELF/entsize.ll.

SGTM, if the code can be tested using llvm-mc.

[nickdesaulniers]

Posted fix for test: https://reviews.llvm.org/D53146

lib/CodeGen/TargetLoweringObjectFileImpl.cpp

static const MCSymbolELF *getAssociatedSymbol(const GlobalObject *GO,
   auto *VM = dyn_cast<ValueAsMetadata>(Op);
   if (!VM)
     report_fatal_error("MD_associated operand is not ValueAsMetadata");
 
   GlobalObject *OtherGO = dyn_cast<GlobalObject>(VM->getValue());
   return OtherGO ? dyn_cast<MCSymbolELF>(TM.getSymbol(OtherGO)) : nullptr;
 }
 
+static unsigned getEntrySizeForKind(SectionKind Kind) {
+  if (Kind.isMergeable1ByteCString())
+    return 1;
+  else if (Kind.isMergeable2ByteCString())
+    return 2;
+  else if (Kind.isMergeable4ByteCString())
+    return 4;
+  else if (Kind.isMergeableConst4())
+    return 4;
+  else if (Kind.isMergeableConst8())
+    return 8;
+  else if (Kind.isMergeableConst16())
+    return 16;
+  else if (Kind.isMergeableConst32())
+    return 32;
+  else {
+    // We shouldn't have mergeable C strings or mergeable constants that we
+    // didn't handle above.
+    assert(!Kind.isMergeableCString() && "unknown string width");
+    assert(!Kind.isMergeableConst() && "unknown data width");
+    return 0;
+  }
+}
+
 MCSection *TargetLoweringObjectFileELF::getExplicitSectionGlobal(
     const GlobalObject *GO, SectionKind Kind, const TargetMachine &TM) const {
   StringRef SectionName = GO->getSection();
 
   // Check if '#pragma clang section' name is applicable.
   // Note that pragma directive overrides -ffunction-section, -fdata-section
   // and so section name is exactly as user specified and not uniqued.
   const GlobalVariable *GV = dyn_cast<GlobalVariable>(GO);
MCSection *TargetLoweringObjectFileELF::getExplicitSectionGlobal(
   const MCSymbolELF *AssociatedSymbol = getAssociatedSymbol(GO, TM);
   if (AssociatedSymbol) {
     UniqueID = NextUniqueID++;
     Flags |= ELF::SHF_LINK_ORDER;
   }
 
   MCSectionELF *Section = getContext().getELFSection(
       SectionName, getELFSectionType(SectionName, Kind), Flags,
-      /*EntrySize=*/0, Group, UniqueID, AssociatedSymbol);
+      getEntrySizeForKind(Kind), Group, UniqueID, AssociatedSymbol);
   // Make sure that we did not get some other section with incompatible sh_link.
   // This should not be possible due to UniqueID code above.
   assert(Section->getAssociatedSymbol() == AssociatedSymbol &&
          "Associated symbol mismatch between sections");
   return Section;
 }
 
 /// Return the section prefix name used by options FunctionsSections and
static StringRef getSectionPrefixForGlobal(SectionKind Kind) {
   if (Kind.isThreadBSS())
     return ".tbss";
   if (Kind.isData())
     return ".data";
   assert(Kind.isReadOnlyWithRel() && "Unknown section kind");
   return ".data.rel.ro";
 }
 
-static unsigned getEntrySizeForKind(SectionKind Kind) {
-  if (Kind.isMergeable1ByteCString())
-    return 1;
-  else if (Kind.isMergeable2ByteCString())
-    return 2;
-  else if (Kind.isMergeable4ByteCString())
-    return 4;
-  else if (Kind.isMergeableConst4())
-    return 4;
-  else if (Kind.isMergeableConst8())
-    return 8;
-  else if (Kind.isMergeableConst16())
-    return 16;
-  else if (Kind.isMergeableConst32())
-    return 32;
-  else {
-    // We shouldn't have mergeable C strings or mergeable constants that we
-    // didn't handle above.
-    assert(!Kind.isMergeableCString() && "unknown string width");
-    assert(!Kind.isMergeableConst() && "unknown data width");
-    return 0;
-  }
-}
-
 static MCSectionELF *selectELFSectionForGlobal(
     MCContext &Ctx, const GlobalObject *GO, SectionKind Kind, Mangler &Mang,
     const TargetMachine &TM, bool EmitUniqueSection, unsigned Flags,
     unsigned *NextUniqueID, const MCSymbolELF *AssociatedSymbol) {
 
   StringRef Group = "";
   if (const Comdat *C = getELFComdat(GO)) {
     Flags |= ELF::SHF_GROUP;

test/CodeGen/Generic/section_mergeable_size.ll

+; RUN: llc < %s | FileCheck %s
+@a = internal unnamed_addr constant [1 x [1 x i32]] zeroinitializer, section ".init.rodata", align 4
+; CHECK: .init.rodata,"aM",@progbits,4