[llvm] r338018 - Revert "[COFF] Use comdat shared constants for MinGW as well"

Martin Storsjo via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 26 03:48:21 PDT 2018


Author: mstorsjo
Date: Thu Jul 26 03:48:20 2018
New Revision: 338018

URL: http://llvm.org/viewvc/llvm-project?rev=338018&view=rev
Log:
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"
        ^
<inline asm>:1:15: note: instantiated into assembly here
        imull __real at 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.

Modified:
    llvm/trunk/include/llvm/MC/MCAsmInfo.h
    llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
    llvm/trunk/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
    llvm/trunk/lib/MC/MCAsmInfoCOFF.cpp
    llvm/trunk/test/CodeGen/AArch64/win_cst_pool.ll
    llvm/trunk/test/CodeGen/X86/win_cst_pool.ll

Modified: llvm/trunk/include/llvm/MC/MCAsmInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCAsmInfo.h?rev=338018&r1=338017&r2=338018&view=diff
==============================================================================
--- llvm/trunk/include/llvm/MC/MCAsmInfo.h (original)
+++ llvm/trunk/include/llvm/MC/MCAsmInfo.h Thu Jul 26 03:48:20 2018
@@ -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; }

Modified: llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp?rev=338018&r1=338017&r2=338018&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (original)
+++ llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp Thu Jul 26 03:48:20 2018
@@ -2664,7 +2664,7 @@ MCSymbol *AsmPrinter::GetBlockAddressSym
 
 /// 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()) {

Modified: llvm/trunk/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/TargetLoweringObjectFileImpl.cpp?rev=338018&r1=338017&r2=338018&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/TargetLoweringObjectFileImpl.cpp (original)
+++ llvm/trunk/lib/CodeGen/TargetLoweringObjectFileImpl.cpp Thu Jul 26 03:48:20 2018
@@ -1396,7 +1396,12 @@ static std::string scalarConstantToHexSt
 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;

Modified: llvm/trunk/lib/MC/MCAsmInfoCOFF.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAsmInfoCOFF.cpp?rev=338018&r1=338017&r2=338018&view=diff
==============================================================================
--- llvm/trunk/lib/MC/MCAsmInfoCOFF.cpp (original)
+++ llvm/trunk/lib/MC/MCAsmInfoCOFF.cpp Thu Jul 26 03:48:20 2018
@@ -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;
 }

Modified: llvm/trunk/test/CodeGen/AArch64/win_cst_pool.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/win_cst_pool.ll?rev=338018&r1=338017&r2=338018&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/win_cst_pool.ll (original)
+++ llvm/trunk/test/CodeGen/AArch64/win_cst_pool.ll Thu Jul 26 03:48:20 2018
@@ -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 at 0000000000800000
 ; CHECK-NEXT:          ldr     d0, [x8, __real at 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

Modified: llvm/trunk/test/CodeGen/X86/win_cst_pool.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/win_cst_pool.ll?rev=338018&r1=338017&r2=338018&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/win_cst_pool.ll (original)
+++ llvm/trunk/test/CodeGen/X86/win_cst_pool.ll Thu Jul 26 03:48:20 2018
@@ -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 at 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> <i32 3, i32 2, i32 1, i32 0>
 }




More information about the llvm-commits mailing list