[MC] When using bundle aligment, align sections to bundle size

[dschuff]

Summary

Bundle aligment requires that the functions always start at an aligned address.
Usually this is ensured by the compiler, but assembly code does not always
begin with a .align directive.

This change ensures that sections get the correct alignment if they contain
any instructions and bundling is enabled. (It also makes LLVM match the
behavior of GNU as).

[• rafael]

Why do you need to call it from both locations?

Needs a testcase.

lib/MC/MCELFStreamer.cpp

134 ↗

(On Diff #24069)

Start function names with a lowercase letter.

[dschuff]

see comment in the test plan section about testing. Should I just change llvm-objdump to print the alignment of each section so I can add a test?

wrt calling it from both places:
This needs to happen for every section that has instructions, so it has to be on switching away from each section, and on finish (unless the last section is guaranteed to be switched away from before FinishImpl runs?)
Alternatively, I could just iterate over all the sections in some finish function, but I didn't see a way to get them from here. Maybe in MCAssembler::Finish?

lib/MC/MCELFStreamer.cpp

134 ↗

(On Diff #24069)

Done.

[dschuff]

• use lowercase function name

[jvoung]

How about use "llvm-readobj -sections" to test? That will print out the sections plus the "AddressAlignment".

[dschuff]

• Add test

[dschuff]

In D9131#158716, @jvoung wrote:

How about use "llvm-readobj -sections" to test? That will print out the sections plus the "AddressAlignment".

ah, didn't even know about that. Done.

[• rafael]

LGTM with a comment on each call on why it is there.

test/MC/X86/AlignedBundling/section-alignment.s

9 ↗

(On Diff #24075)

Add a
CHECK-NOT: Name

to make sure it is matching the correct section.

[dschuff]

• add comments and tighten test

test/MC/X86/AlignedBundling/section-alignment.s

9 ↗

(On Diff #24075)

Done.

[• rafael]

LGTM with test update

test/MC/X86/AlignedBundling/section-alignment.s

16 ↗

(On Diff #24083)

CHECK-NOT in here too.

20 ↗

(On Diff #24083)

and here

llvm/trunk/lib/MC/MCELFStreamer.cpp

void MCELFStreamer::EmitAssemblerFlag(MCAssemblerFlag Flag) {
   case MCAF_SubsectionsViaSymbols:
     getAssembler().setSubsectionsViaSymbols(true);
     return;
   }
 
   llvm_unreachable("invalid assembler flag!");
 }
 
+// If bundle aligment is used and there are any instructions in the section, it
+// needs to be aligned to at least the bundle size.
+static void setSectionAlignmentForBundling(
+    const MCAssembler &Assembler, MCSectionData *Section) {
+  if (Assembler.isBundlingEnabled() && Section &&
+      Section->hasInstructions() &&
+      Section->getAlignment() < Assembler.getBundleAlignSize())
+    Section->setAlignment(Assembler.getBundleAlignSize());
+}
+
 void MCELFStreamer::ChangeSection(const MCSection *Section,
                                   const MCExpr *Subsection) {
   MCSectionData *CurSection = getCurrentSectionData();
   if (CurSection && CurSection->isBundleLocked())
     report_fatal_error("Unterminated .bundle_lock when changing a section");
 
   MCAssembler &Asm = getAssembler();
+  // Ensure the previous section gets aligned if necessary.
+  setSectionAlignmentForBundling(Asm, CurSection);
   auto *SectionELF = static_cast<const MCSectionELF *>(Section);
   const MCSymbol *Grp = SectionELF->getGroup();
   if (Grp)
     Asm.getOrCreateSymbolData(*Grp);
 
   this->MCObjectStreamer::ChangeSection(Section, Subsection);
   MCSymbol *SectionSymbol = getContext().getOrCreateSectionSymbol(*SectionELF);
   if (SectionSymbol->isUndefined()) {
for (std::vector<LocalCommon>::const_iterator i = LocalCommons.begin(),
     if (ByteAlignment > SectData.getAlignment())
       SectData.setAlignment(ByteAlignment);
   }
 
   LocalCommons.clear();
 }
 
 void MCELFStreamer::FinishImpl() {
+  // Ensure the last section gets aligned if necessary.
+  setSectionAlignmentForBundling(getAssembler(), getCurrentSectionData());
+
   EmitFrames(nullptr);
 
   Flush();
 
   this->MCObjectStreamer::FinishImpl();
 }
 
 MCStreamer *llvm::createELFStreamer(MCContext &Context, MCAsmBackend &MAB,

llvm/trunk/test/MC/X86/AlignedBundling/section-alignment.s

+# RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o - \
+# RUN:   | llvm-readobj -sections | FileCheck %s
+
+# Test that bundle-aligned sections with instructions are aligned
+
+  .bundle_align_mode 5
+# CHECK: Sections
+# Check that the empty .text section has the default alignment
+# CHECK-LABEL: Name: .text
+# CHECK-NOT: Name
+# CHECK: AddressAlignment: 4
+
+  .section text1, "x"
+  imull $17, %ebx, %ebp
+# CHECK-LABEL: Name: text1
+# CHECK: AddressAlignment: 32
+
+  .section text2, "x"
+  imull $17, %ebx, %ebp
+# CHECK-LABEL: Name: text2
+# CHECK: AddressAlignment: 32