From a3ea407d48eba34c712399c8885a49861ea9a741 Mon Sep 17 00:00:00 2001 From: David Majnemer Date: Sun, 21 Feb 2016 01:30:30 +0000 Subject: [PATCH] [X86] Use the correct alignment for COMDAT constant pool entries COFF doesn't have sections with mergeable contents. Instead, each constant pool entry ends up in a COMDAT section. The linker, when choosing between COMDAT sections, doesn't choose the max alignment of the two sections. You just get whatever alignment was on the section. If one constant needed a higher alignment in one object file from another one, then we will get into trouble if the linker chooses the lower alignment one. Instead, lets promote the alignment of the constant pool entry to make sure we don't use an under aligned constant with an instruction which assumed otherwise. This fixes PR26680. llvm-svn: 261462 --- .../llvm/CodeGen/TargetLoweringObjectFileImpl.h | 6 ++++-- .../include/llvm/Target/TargetLoweringObjectFile.h | 3 ++- llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp | 7 +++--- llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp | 6 ++++-- llvm/lib/Target/TargetLoweringObjectFile.cpp | 7 ++++-- llvm/lib/Target/X86/X86AsmPrinter.cpp | 3 ++- llvm/lib/Target/X86/X86TargetObjectFile.cpp | 25 ++++++++++++++++------ llvm/lib/Target/X86/X86TargetObjectFile.h | 3 ++- llvm/test/CodeGen/X86/win_cst_pool.ll | 2 +- 9 files changed, 43 insertions(+), 19 deletions(-) diff --git a/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h b/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h index 2f13791..61dbc00 100644 --- a/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h +++ b/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h @@ -47,7 +47,8 @@ public: /// Given a constant with the SectionKind, return a section that it should be /// placed in. MCSection *getSectionForConstant(const DataLayout &DL, SectionKind Kind, - const Constant *C) const override; + const Constant *C, + unsigned &Align) const override; MCSection *getExplicitSectionGlobal(const GlobalValue *GV, SectionKind Kind, Mangler &Mang, @@ -104,7 +105,8 @@ public: const TargetMachine &TM) const override; MCSection *getSectionForConstant(const DataLayout &DL, SectionKind Kind, - const Constant *C) const override; + const Constant *C, + unsigned &Align) const override; /// The mach-o version of this method defaults to returning a stub reference. const MCExpr * diff --git a/llvm/include/llvm/Target/TargetLoweringObjectFile.h b/llvm/include/llvm/Target/TargetLoweringObjectFile.h index cb52698..2f9dace 100644 --- a/llvm/include/llvm/Target/TargetLoweringObjectFile.h +++ b/llvm/include/llvm/Target/TargetLoweringObjectFile.h @@ -71,7 +71,8 @@ public: /// placed in. virtual MCSection *getSectionForConstant(const DataLayout &DL, SectionKind Kind, - const Constant *C) const; + const Constant *C, + unsigned &Align) const; /// Classify the specified global variable into a set of target independent /// categories embodied in SectionKind. diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp index 4d945bd..47f21b3 100644 --- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp @@ -1194,9 +1194,10 @@ bool AsmPrinter::doFinalization(Module &M) { // Emit __morestack address if needed for indirect calls. if (MMI->usesMorestackAddr()) { + unsigned Align = 1; MCSection *ReadOnlySection = getObjFileLowering().getSectionForConstant( getDataLayout(), SectionKind::getReadOnly(), - /*C=*/nullptr); + /*C=*/nullptr, Align); OutStreamer->SwitchSection(ReadOnlySection); MCSymbol *AddrSymbol = @@ -1286,8 +1287,8 @@ void AsmPrinter::EmitConstantPool() { if (!CPE.isMachineConstantPoolEntry()) C = CPE.Val.ConstVal; - MCSection *S = - getObjFileLowering().getSectionForConstant(getDataLayout(), Kind, C); + MCSection *S = getObjFileLowering().getSectionForConstant(getDataLayout(), + Kind, C, Align); // The number of sections are small, just do a linear search from the // last section to the first. diff --git a/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp b/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp index 5dfdb7a..81b429a 100644 --- a/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp +++ b/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp @@ -351,7 +351,8 @@ bool TargetLoweringObjectFileELF::shouldPutJumpTableInFunctionSection( /// Given a mergeable constant with the specified size and relocation /// information, return a section that it should be placed in. MCSection *TargetLoweringObjectFileELF::getSectionForConstant( - const DataLayout &DL, SectionKind Kind, const Constant *C) const { + const DataLayout &DL, SectionKind Kind, const Constant *C, + unsigned &Align) const { if (Kind.isMergeableConst4() && MergeableConst4Section) return MergeableConst4Section; if (Kind.isMergeableConst8() && MergeableConst8Section) @@ -636,7 +637,8 @@ MCSection *TargetLoweringObjectFileMachO::SelectSectionForGlobal( } MCSection *TargetLoweringObjectFileMachO::getSectionForConstant( - const DataLayout &DL, SectionKind Kind, const Constant *C) const { + const DataLayout &DL, SectionKind Kind, const Constant *C, + unsigned &Align) const { // If this constant requires a relocation, we have to put it in the data // segment, not in the text segment. if (Kind.isData() || Kind.isReadOnlyWithRel()) diff --git a/llvm/lib/Target/TargetLoweringObjectFile.cpp b/llvm/lib/Target/TargetLoweringObjectFile.cpp index a0b0d8f..3c04a21 100644 --- a/llvm/lib/Target/TargetLoweringObjectFile.cpp +++ b/llvm/lib/Target/TargetLoweringObjectFile.cpp @@ -252,8 +252,10 @@ TargetLoweringObjectFile::SectionForGlobal(const GlobalValue *GV, MCSection *TargetLoweringObjectFile::getSectionForJumpTable( const Function &F, Mangler &Mang, const TargetMachine &TM) const { + unsigned Align = 0; return getSectionForConstant(F.getParent()->getDataLayout(), - SectionKind::getReadOnly(), /*C=*/nullptr); + SectionKind::getReadOnly(), /*C=*/nullptr, + Align); } bool TargetLoweringObjectFile::shouldPutJumpTableInFunctionSection( @@ -277,7 +279,8 @@ bool TargetLoweringObjectFile::shouldPutJumpTableInFunctionSection( /// Given a mergable constant with the specified size and relocation /// information, return a section that it should be placed in. MCSection *TargetLoweringObjectFile::getSectionForConstant( - const DataLayout &DL, SectionKind Kind, const Constant *C) const { + const DataLayout &DL, SectionKind Kind, const Constant *C, + unsigned &Align) const { if (Kind.isReadOnly() && ReadOnlySection != nullptr) return ReadOnlySection; diff --git a/llvm/lib/Target/X86/X86AsmPrinter.cpp b/llvm/lib/Target/X86/X86AsmPrinter.cpp index 2170e62..b951398 100644 --- a/llvm/lib/Target/X86/X86AsmPrinter.cpp +++ b/llvm/lib/Target/X86/X86AsmPrinter.cpp @@ -568,8 +568,9 @@ MCSymbol *X86AsmPrinter::GetCPISymbol(unsigned CPID) const { const DataLayout &DL = MF->getDataLayout(); SectionKind Kind = CPE.getSectionKind(&DL); const Constant *C = CPE.Val.ConstVal; + unsigned Align = CPE.Alignment; if (const MCSectionCOFF *S = dyn_cast( - getObjFileLowering().getSectionForConstant(DL, Kind, C))) { + getObjFileLowering().getSectionForConstant(DL, Kind, C, Align))) { if (MCSymbol *Sym = S->getCOMDATSymbol()) { if (Sym->isUndefined()) OutStreamer->EmitSymbolAttribute(Sym, MCSA_Global); diff --git a/llvm/lib/Target/X86/X86TargetObjectFile.cpp b/llvm/lib/Target/X86/X86TargetObjectFile.cpp index 782768d..0d37fcd 100644 --- a/llvm/lib/Target/X86/X86TargetObjectFile.cpp +++ b/llvm/lib/Target/X86/X86TargetObjectFile.cpp @@ -154,16 +154,29 @@ static std::string scalarConstantToHexString(const Constant *C) { } MCSection *X86WindowsTargetObjectFile::getSectionForConstant( - const DataLayout &DL, SectionKind Kind, const Constant *C) const { + const DataLayout &DL, SectionKind Kind, const Constant *C, + unsigned &Align) const { if (Kind.isMergeableConst() && C) { const unsigned Characteristics = COFF::IMAGE_SCN_CNT_INITIALIZED_DATA | COFF::IMAGE_SCN_MEM_READ | COFF::IMAGE_SCN_LNK_COMDAT; std::string COMDATSymName; - if (Kind.isMergeableConst4() || Kind.isMergeableConst8()) - COMDATSymName = "__real@" + scalarConstantToHexString(C); - else if (Kind.isMergeableConst16()) - COMDATSymName = "__xmm@" + scalarConstantToHexString(C); + if (Kind.isMergeableConst4()) { + if (Align <= 4) { + COMDATSymName = "__real@" + scalarConstantToHexString(C); + Align = 4; + } + } else if (Kind.isMergeableConst8()) { + if (Align <= 8) { + COMDATSymName = "__real@" + scalarConstantToHexString(C); + Align = 8; + } + } else if (Kind.isMergeableConst16()) { + if (Align <= 16) { + COMDATSymName = "__xmm@" + scalarConstantToHexString(C); + Align = 16; + } + } if (!COMDATSymName.empty()) return getContext().getCOFFSection(".rdata", Characteristics, Kind, @@ -171,5 +184,5 @@ MCSection *X86WindowsTargetObjectFile::getSectionForConstant( COFF::IMAGE_COMDAT_SELECT_ANY); } - return TargetLoweringObjectFile::getSectionForConstant(DL, Kind, C); + return TargetLoweringObjectFile::getSectionForConstant(DL, Kind, C, Align); } diff --git a/llvm/lib/Target/X86/X86TargetObjectFile.h b/llvm/lib/Target/X86/X86TargetObjectFile.h index 6b2448c..81207ad 100644 --- a/llvm/lib/Target/X86/X86TargetObjectFile.h +++ b/llvm/lib/Target/X86/X86TargetObjectFile.h @@ -59,7 +59,8 @@ namespace llvm { /// \brief Given a mergeable constant with the specified size and relocation /// information, return a section that it should be placed in. MCSection *getSectionForConstant(const DataLayout &DL, SectionKind Kind, - const Constant *C) const override; + const Constant *C, + unsigned &Align) const override; }; } // end namespace llvm diff --git a/llvm/test/CodeGen/X86/win_cst_pool.ll b/llvm/test/CodeGen/X86/win_cst_pool.ll index e8624a6..2856a08 100644 --- a/llvm/test/CodeGen/X86/win_cst_pool.ll +++ b/llvm/test/CodeGen/X86/win_cst_pool.ll @@ -73,7 +73,7 @@ define float @pr23966(i32 %a) { ; CHECK: .globl __real@bf8000003f800000 ; CHECK-NEXT: .section .rdata,"dr",discard,__real@bf8000003f800000 -; CHECK-NEXT: .p2align 2 +; CHECK-NEXT: .p2align 3 ; CHECK-NEXT: __real@bf8000003f800000: ; CHECK-NEXT: .long 1065353216 ; CHECK-NEXT: .long 3212836864 -- 2.7.4