[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

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

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

lib/MC/MCELFStreamer.cpp

void MCELFStreamer::EmitAssemblerFlag(MCAssemblerFlag Flag) {
   case MCAF_SubsectionsViaSymbols:
     getAssembler().setSubsectionsViaSymbols(true);
     return;
   }
 
   llvm_unreachable("invalid assembler flag!");
 }
 
+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();
+  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() {
+  SetSectionAlignmentForBundling(getAssembler(), getCurrentSectionData());
+
   EmitFrames(nullptr);
 
   Flush();
 
   this->MCObjectStreamer::FinishImpl();
 }
 
 MCStreamer *llvm::createELFStreamer(MCContext &Context, MCAsmBackend &MAB,