[NFC] Transition GlobalObject alignment from MaybeAlign to Align

[gchatelet]

Summary

This is a follow up on https://reviews.llvm.org/D142459#4081179.
This first patch adds an overload to GlobalObject::setAlignment that accepts an Align type.
This already handles most of the calls.

This patch also converts a few call sites to the new type when this is safe.

Here is the list of the remaining call sites:

• clang/lib/CodeGen/CodeGenModule.cpp:1688
• llvm/lib/AsmParser/LLParser.cpp:1309
• llvm/lib/AsmParser/LLParser.cpp:6050
• llvm/lib/Bitcode/Reader/BitcodeReader.cpp:3871
• llvm/lib/Bitcode/Reader/BitcodeReader.cpp:4030
• llvm/lib/IR/Core.cpp:2018
• llvm/lib/IR/Globals.cpp:141
• llvm/lib/Linker/IRMover.cpp:660
• llvm/lib/Linker/LinkModules.cpp:361
• llvm/lib/Linker/LinkModules.cpp:362
• llvm/lib/Transforms/IPO/MergeFunctions.cpp:782
• llvm/lib/Transforms/IPO/MergeFunctions.cpp:840
• llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1813
• llvm/tools/llvm-reduce/deltas/ReduceGlobalObjects.cpp:27

[courbet]

This one is NFC, tag it as such ?

llvm/include/llvm/IR/GlobalObject.h

90

defined*

[gchatelet]

• rebase, fix typo and amend commit message to add [NFC]

[gchatelet]

@efriedma regarding the remaining calls, some of them will autosolve when getAlign will return Align instead of MaybeAlign but there are a few ones for which I'm unsure how to proceed.
More particularly the following ones that deal with creating new GlobalObjects. Is this reasonable to align to 1 when unknown or can we do better?

• llvm/lib/AsmParser/LLParser.cpp:1309
• llvm/lib/AsmParser/LLParser.cpp:6050
• llvm/lib/Bitcode/Reader/BitcodeReader.cpp:3871
• llvm/lib/Bitcode/Reader/BitcodeReader.cpp:4030

[nikic]

@gchatelet I believe you'll want to use the preferred type alignment. Looking at https://github.com/llvm/llvm-project/blob/72121a20cda4dc91d0ef5548f93046e71c5ec6f6/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp#L324, we will currently pick the preferred type alignment as the default.

[gchatelet]

In D142708#4085604, @nikic wrote:

@gchatelet I believe you'll want to use the preferred type alignment. Looking at https://github.com/llvm/llvm-project/blob/72121a20cda4dc91d0ef5548f93046e71c5ec6f6/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp#L324, we will currently pick the preferred type alignment as the default.

Thx for the suggestion @nikic!
If I manually apply dead code elimination to the code you're referring to with GV being a Function and GV->getAlign() returning std::nullopt I have the following code

Align AsmPrinter::getGVAlignment(const GlobalObject *GV, const DataLayout &DL, Align InAlign = Align(1)) {
  Align Alignment;
  if (InAlign > Alignment)
    Alignment = InAlign;
  return Alignment;
}

Which is basically std::max(InAlign, Align(1)) or simply Align(1) as I don't see an obvious value for InAlign other than 1.

So 1 it is?

[gchatelet]

fix formatting

[efriedma]

In D142708#4085789, @gchatelet wrote:

Which is basically std::max(InAlign, Align(1)) or simply Align(1) as I don't see an obvious value for InAlign other than 1.

Some callers pass in other values.

llvm/include/llvm/IR/GlobalObject.h

86

Not sure I understand the point of overloading setAlignment; there's an implicit conversion from Align to MaybeAlign.

[gchatelet]

In D142708#4086420, @efriedma wrote:

In D142708#4085789, @gchatelet wrote:

Which is basically std::max(InAlign, Align(1)) or simply Align(1) as I don't see an obvious value for InAlign other than 1.

Some callers pass in other values.

Yes indeed, the only call to this function specifying the InAlign argument is this one, appearing in AsmPrinter::emitAlignment.
Unfortunately this one is called a lot (32 instances). There are some arch specific tunings as well :-/

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:810:      emitAlignment(Alignment, GV);
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:846:  emitAlignment(Alignment, GV);
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:903:    emitAlignment(MF->getAlignment(), &F);
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2145:      emitAlignment(Align(DL.getPointerSize()));
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2172:        emitAlignment(Align(DL.getPointerSize()));
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2452:        emitAlignment(Align(CPSections[i].Alignment));
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2497:  emitAlignment(Align(MJTI->getEntryAlignment(DL)));
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2732:      emitAlignment(Align);
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2830:void AsmPrinter::emitAlignment(Align Alignment, const GlobalObject *GV,
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:3691:    emitAlignment(Alignment, nullptr, MBB.getMaxBytesForAlignment());
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:4021:    emitAlignment(Align(PointerSize));
llvm/lib/CodeGen/AsmPrinter/EHStreamer.cpp:473:  Asm->emitAlignment(Align(4));
llvm/lib/CodeGen/AsmPrinter/EHStreamer.cpp:673:        Asm->emitAlignment(Align(4));
llvm/lib/CodeGen/AsmPrinter/EHStreamer.cpp:802:    Asm->emitAlignment(Align(4));
llvm/lib/CodeGen/AsmPrinter/EHStreamer.cpp:806:  Asm->emitAlignment(Align(4));
llvm/lib/CodeGen/AsmPrinter/ErlangGCPrinter.cpp:72:    AP.emitAlignment(IntPtrSize == 4 ? Align(4) : Align(8));
llvm/lib/CodeGen/AsmPrinter/OcamlGCPrinter.cpp:128:  AP.emitAlignment(IntPtrSize == 4 ? Align(4) : Align(8));
llvm/lib/CodeGen/AsmPrinter/OcamlGCPrinter.cpp:179:      AP.emitAlignment(IntPtrSize == 4 ? Align(4) : Align(8));
llvm/lib/CodeGen/AsmPrinter/WinException.cpp:208:    Asm->emitAlignment(std::max(Asm->MF->getAlignment(), MBB.getAlignment()),
llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp:946:    emitAlignment(Align(Size));
llvm/lib/Target/ARM/ARMAsmPrinter.cpp:179:    emitAlignment(Align(2));
llvm/lib/Target/ARM/ARMAsmPrinter.cpp:537:      emitAlignment(Align(4));
llvm/lib/Target/ARM/ARMAsmPrinter.cpp:550:      emitAlignment(Align(4));
llvm/lib/Target/ARM/ARMAsmPrinter.cpp:983:  emitAlignment(Align(4));
llvm/lib/Target/ARM/ARMAsmPrinter.cpp:1029:  emitAlignment(Align(4));
llvm/lib/Target/ARM/ARMAsmPrinter.cpp:1058:    emitAlignment(Align(4));
llvm/lib/Target/ARM/ARMAsmPrinter.cpp:1101:  emitAlignment(Align(2));
llvm/lib/Target/Mips/MipsAsmPrinter.cpp:406:    emitAlignment(std::max(MF->getAlignment(), MIPS_NACL_BUNDLE_ALIGN));
llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:2443:  emitAlignment(getGVAlignment(GV, DL), GV);
llvm/lib/Target/X86/X86AsmPrinter.cpp:774:      emitAlignment(WordSize == 4 ? Align(4) : Align(8));
llvm/lib/Target/X86/X86AsmPrinter.cpp:784:      emitAlignment(WordSize == 4 ? Align(4) : Align(8)); // padding
llvm/lib/Target/XCore/XCoreAsmPrinter.cpp:145:  emitAlignment(std::max(Alignment, Align(4)), GV);

llvm/include/llvm/IR/GlobalObject.h

86

The purpose here is to define a deprecation path for downstream users.
Once we've cleaned up calls in the LLVM repo we will mark the void setAlignment(MaybeAlign Align); with LLVM_DEPRECATED to help downstream users migrate to the new API.
It also helps catch all call sites by marking the function deprecated and compile the codebase with -DLLVM_ENABLE_WERROR=ON

[gchatelet]

I'm submitting this now as it is NFC.
I'll follow up on a case by case basis.

[gchatelet]

Breakage came from GCC being more strict on type redefinition

/b/sanitizer-x86_64-linux/build/llvm-project/llvm/include/llvm/LTO/LTO.h:292:13: error: declaration of ‘llvm::Align llvm::lto::LTO::RegularLTOState::CommonResolution::Align’ changes meaning of ‘Align’ [-fpermissive]
  292 |       Align Align;
      |             ^~~~~

I changed the variable name to Alignment instead.