From 9dafd6f6d93ca67f63c09aa091b28ddf88ddfc14 Mon Sep 17 00:00:00 2001 From: Martin Storsjo Date: Thu, 26 Jul 2018 10:48:20 +0000 Subject: [PATCH] Revert "[COFF] Use comdat shared constants for MinGW as well" This reverts commit r337951. While that kind of shared constant generally works fine in a MinGW setting, it broke some cases of inline assembly that worked before: $ cat const-asm.c int MULH(int a, int b) { int rt, dummy; __asm__ ( "imull %3" :"=d"(rt), "=a"(dummy) :"a"(a), "rm"(b) ); return rt; } int func(int a) { return MULH(a, 1); } $ clang -target x86_64-win32-gnu -c const-asm.c -O2 const-asm.c:4:9: error: invalid variant '00000001' "imull %3" ^ :1:15: note: instantiated into assembly here imull __real@00000001(%rip) ^ A similar error is produced for i686 as well. The same test with a target of x86_64-win32-msvc or i686-win32-msvc works fine. llvm-svn: 338018 --- llvm/include/llvm/MC/MCAsmInfo.h | 5 +++++ llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp | 2 +- llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp | 7 ++++++- llvm/lib/MC/MCAsmInfoCOFF.cpp | 8 ++++++++ llvm/test/CodeGen/AArch64/win_cst_pool.ll | 11 ++++++++++- llvm/test/CodeGen/X86/win_cst_pool.ll | 10 +++++++++- 6 files changed, 39 insertions(+), 4 deletions(-) diff --git a/llvm/include/llvm/MC/MCAsmInfo.h b/llvm/include/llvm/MC/MCAsmInfo.h index 0a0f379..120fb8f 100644 --- a/llvm/include/llvm/MC/MCAsmInfo.h +++ b/llvm/include/llvm/MC/MCAsmInfo.h @@ -89,6 +89,10 @@ protected: /// them. bool HasCOFFAssociativeComdats = false; + /// True if this is a non-GNU COFF target. For GNU targets, we don't generate + /// constants into comdat sections. + bool HasCOFFComdatConstants = false; + /// This is the maximum possible length of an instruction, which is needed to /// compute the size of an inline asm. Defaults to 4. unsigned MaxInstLength = 4; @@ -469,6 +473,7 @@ public: bool hasMachoZeroFillDirective() const { return HasMachoZeroFillDirective; } bool hasMachoTBSSDirective() const { return HasMachoTBSSDirective; } bool hasCOFFAssociativeComdats() const { return HasCOFFAssociativeComdats; } + bool hasCOFFComdatConstants() const { return HasCOFFComdatConstants; } unsigned getMaxInstLength() const { return MaxInstLength; } unsigned getMinInstAlignment() const { return MinInstAlignment; } bool getDollarIsPC() const { return DollarIsPC; } diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp index f659282..9bbc77b 100644 --- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp @@ -2664,7 +2664,7 @@ MCSymbol *AsmPrinter::GetBlockAddressSymbol(const BasicBlock *BB) const { /// GetCPISymbol - Return the symbol for the specified constant pool entry. MCSymbol *AsmPrinter::GetCPISymbol(unsigned CPID) const { - if (getSubtargetInfo().getTargetTriple().isOSWindows()) { + if (getSubtargetInfo().getTargetTriple().isKnownWindowsMSVCEnvironment()) { const MachineConstantPoolEntry &CPE = MF->getConstantPool()->getConstants()[CPID]; if (!CPE.isMachineConstantPoolEntry()) { diff --git a/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp b/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp index 149cec2..b5dd2d4c 100644 --- a/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp +++ b/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp @@ -1396,7 +1396,12 @@ static std::string scalarConstantToHexString(const Constant *C) { MCSection *TargetLoweringObjectFileCOFF::getSectionForConstant( const DataLayout &DL, SectionKind Kind, const Constant *C, unsigned &Align) const { - if (Kind.isMergeableConst() && C) { + if (Kind.isMergeableConst() && C && + getContext().getAsmInfo()->hasCOFFComdatConstants()) { + // This creates comdat sections with the given symbol name, but unless + // AsmPrinter::GetCPISymbol actually makes the symbol global, the symbol + // will be created with a null storage class, which makes GNU binutils + // error out. const unsigned Characteristics = COFF::IMAGE_SCN_CNT_INITIALIZED_DATA | COFF::IMAGE_SCN_MEM_READ | COFF::IMAGE_SCN_LNK_COMDAT; diff --git a/llvm/lib/MC/MCAsmInfoCOFF.cpp b/llvm/lib/MC/MCAsmInfoCOFF.cpp index 8bb02c3..d8fb875 100644 --- a/llvm/lib/MC/MCAsmInfoCOFF.cpp +++ b/llvm/lib/MC/MCAsmInfoCOFF.cpp @@ -45,6 +45,11 @@ MCAsmInfoCOFF::MCAsmInfoCOFF() { // If this is a COFF target, assume that it supports associative comdats. It's // part of the spec. HasCOFFAssociativeComdats = true; + + // We can generate constants in comdat sections that can be shared, + // but in order not to create null typed symbols, we actually need to + // make them global symbols as well. + HasCOFFComdatConstants = true; } void MCAsmInfoMicrosoft::anchor() {} @@ -58,4 +63,7 @@ MCAsmInfoGNUCOFF::MCAsmInfoGNUCOFF() { // comdats for jump tables, unwind information, and other data associated with // a function. HasCOFFAssociativeComdats = false; + + // We don't create constants in comdat sections for MinGW. + HasCOFFComdatConstants = false; } diff --git a/llvm/test/CodeGen/AArch64/win_cst_pool.ll b/llvm/test/CodeGen/AArch64/win_cst_pool.ll index 9e78702..5bcc919 100644 --- a/llvm/test/CodeGen/AArch64/win_cst_pool.ll +++ b/llvm/test/CodeGen/AArch64/win_cst_pool.ll @@ -1,5 +1,5 @@ ; RUN: llc < %s -mtriple=aarch64-win32-msvc | FileCheck %s -; RUN: llc < %s -mtriple=aarch64-win32-gnu | FileCheck %s +; RUN: llc < %s -mtriple=aarch64-win32-gnu | FileCheck -check-prefix=MINGW %s define double @double() { ret double 0x0000000000800000 @@ -13,3 +13,12 @@ define double @double() { ; CHECK: adrp x8, __real@0000000000800000 ; CHECK-NEXT: ldr d0, [x8, __real@0000000000800000] ; CHECK-NEXT: ret + +; MINGW: .section .rdata,"dr" +; MINGW-NEXT: .p2align 3 +; MINGW-NEXT: [[LABEL:\.LC.*]]: +; MINGW-NEXT: .xword 8388608 +; MINGW: double: +; MINGW: adrp x8, [[LABEL]] +; MINGW-NEXT: ldr d0, [x8, [[LABEL]]] +; MINGW-NEXT: ret diff --git a/llvm/test/CodeGen/X86/win_cst_pool.ll b/llvm/test/CodeGen/X86/win_cst_pool.ll index 30762e9..cf0aa9b 100644 --- a/llvm/test/CodeGen/X86/win_cst_pool.ll +++ b/llvm/test/CodeGen/X86/win_cst_pool.ll @@ -1,5 +1,5 @@ ; RUN: llc < %s -mtriple=x86_64-win32 -mattr=sse2 -mattr=avx | FileCheck %s -; RUN: llc < %s -mtriple=x86_64-win32-gnu -mattr=sse2 -mattr=avx | FileCheck %s +; RUN: llc < %s -mtriple=x86_64-win32-gnu -mattr=sse2 -mattr=avx | FileCheck -check-prefix=MINGW %s target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" target triple = "x86_64-pc-windows-msvc" @@ -15,6 +15,14 @@ define double @double() { ; CHECK: movsd __real@0000000000800000(%rip), %xmm0 ; CHECK-NEXT: ret +; MINGW: .section .rdata,"dr" +; MINGW-NEXT: .p2align 3 +; MINGW-NEXT: [[LABEL:\.LC.*]]: +; MINGW-NEXT: .quad 8388608 +; MINGW: double: +; MINGW: movsd [[LABEL]](%rip), %xmm0 +; MINGW-NEXT: ret + define <4 x i32> @vec1() { ret <4 x i32> } -- 2.7.4