[lli/COFF] Set the correct alignment for common symbols

[davide]

Summary

Otherwise we set it always to zero, which is not correct, and we assert inside alignTo (Assertion failed: Align != 0u && "Align can't be 0."). Makes lli slightly better on Windows, at least for me =)

[rnk]

lgtm

test/ExecutionEngine/MCJIT/coff-alignment.ll

5–8 ↗

(On Diff #76504)

Want to return the i32 from main so that this isn't fragile to codegen changes?

[davide]

test/ExecutionEngine/MCJIT/coff-alignment.ll

5–8 ↗

(On Diff #76504)

Sure. Thanks for your review, Reid.

[majnemer]

We probably need some logic to handle when the aligncomm directive is present but this is still a good improvement :)

[mboehme]

Looks as if this is causing buildbots to fail:

http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15/builds/215
http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-a15/builds/214
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt/builds/324

llvm/trunk/include/llvm/Object/COFF.h

uint32_t getNumberOfSymbols() const {
     if (!SymbolTable16 && !SymbolTable32)
       return 0;
     return getRawNumberOfSymbols();
   }
 protected:
   void moveSymbolNext(DataRefImpl &Symb) const override;
   Expected<StringRef> getSymbolName(DataRefImpl Symb) const override;
   Expected<uint64_t> getSymbolAddress(DataRefImpl Symb) const override;
+  uint32_t getSymbolAlignment(DataRefImpl Symb) const override;
   uint64_t getSymbolValueImpl(DataRefImpl Symb) const override;
   uint64_t getCommonSymbolSizeImpl(DataRefImpl Symb) const override;
   uint32_t getSymbolFlags(DataRefImpl Symb) const override;
   Expected<SymbolRef::Type> getSymbolType(DataRefImpl Symb) const override;
   Expected<section_iterator> getSymbolSection(DataRefImpl Symb) const override;
   void moveSectionNext(DataRefImpl &Sec) const override;
   std::error_code getSectionName(DataRefImpl Sec,
                                  StringRef &Res) const override;

llvm/trunk/lib/Object/COFFObjectFile.cpp

if (EC)
     return errorCodeToError(EC);
   return Result;
 }
 
 uint64_t COFFObjectFile::getSymbolValueImpl(DataRefImpl Ref) const {
   return getCOFFSymbol(Ref).getValue();
 }
 
+uint32_t COFFObjectFile::getSymbolAlignment(DataRefImpl Ref) const {
+  // MSVC/link.exe seems to align symbols to the next-power-of-2
+  // up to 32 bytes.
+  COFFSymbolRef Symb = getCOFFSymbol(Ref);
+  uint32_t Value = Symb.getValue();
+  return std::min(uint64_t(32),
+                  isPowerOf2_64(Value) ? Value : NextPowerOf2(Value));
+}
+
 Expected<uint64_t> COFFObjectFile::getSymbolAddress(DataRefImpl Ref) const {
   uint64_t Result = getSymbolValue(Ref);
   COFFSymbolRef Symb = getCOFFSymbol(Ref);
   int32_t SectionNumber = Symb.getSectionNumber();
 
   if (Symb.isAnyUndefined() || Symb.isCommon() ||
       COFF::isReservedSectionNumber(SectionNumber))
     return Result;

llvm/trunk/test/ExecutionEngine/MCJIT/coff-alignment.ll

+; RUN: opt -mtriple=x86_64-pc-win32-coff %s -o - | lli
+
+@o = common global i32 0, align 4
+
+define i32 @main() {
+  %patatino = load i32, i32* @o, align 4
+  ret i32 %patatino
+}