[COFF] Fix constant sharing regression for MinGW

[mstorsjo]

Summary

This fixes a regression since SVN r334523, where the object files built targeting MinGW were rejected by GNU binutils tools. Prior to that commit, we only put constants in comdat for MSVC configurations.

[rnk]

include/llvm/CodeGen/TargetLoweringObjectFileImpl.h

137 ↗

(On Diff #152770)

I'd prefer it if we didn't put this here, but instead put something in MCAsmInfo, like the existing HasCOFFAssociativeComdats.

lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1354–1356

It'd be nicer to spell this as a positive check for a gnu or cygwin environment.

[mstorsjo]

include/llvm/CodeGen/TargetLoweringObjectFileImpl.h

137 ↗

(On Diff #152770)

Ok, what'd be a sensible name for it - HasCOFFComdatConstantSections?

lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1354–1356

Yes, it probably would. I matched the condition used in the other function moved in the commit that introduced the issue, but I can change it.

[arm-chrjan01]

LGTM with Reid's suggested changes.

[mstorsjo]

I've pinned down the GNU bfd incompatibility to this:

$ cat const.s
        .section        .rdata,"dr",discard,val
.LCPI0_0:
        .byte 0
$ clang -target x86_64-w64-mingw32 -c const.s -o const.o -c
$ x86_64-w64-mingw32-objdump -d const.o  

const.o:     file format pe-x86-64

x86_64-w64-mingw32-objdump: const.o: Unrecognized storage class 0 for .rdata symbol `val'
x86_64-w64-mingw32-objdump: failed to read symbol table from: const.o
x86_64-w64-mingw32-objdump: error message was: File in wrong format

If I make the symbol val a proper symbol, either by adding a val: in the file, or a .globl val (changing the symbol table entry value from IMAGE_SYM_CLASS_NULL to something else).

I think this maybe even could use the existing flag HasCOFFAssociativeComdats instead of inventing a new one, since it seems to be pretty closely related to the same concept.

[mstorsjo]

Updated to use a flag from MCAsmInfo instead of storing and checking the triplet.

[mstorsjo]

As @rnk doesn't seem to be active at the moment to give this the extra ack (and while this is pretty much what he suggested in the earlier review), I'll go ahead and commit this, since it fixes a regression.

lib/CodeGen/TargetLoweringObjectFileImpl.cpp

       HexString += scalarConstantToHexString(C->getAggregateElement(I));
     return HexString;
   }
 }
 
 MCSection *TargetLoweringObjectFileCOFF::getSectionForConstant(
     const DataLayout &DL, SectionKind Kind, const Constant *C,
     unsigned &Align) const {
-  if (Kind.isMergeableConst() && C) {
+  if (Kind.isMergeableConst() && C &&
+      getContext().getAsmInfo()->hasCOFFAssociativeComdats()) {
+    // GNU binutils doesn't support the kind of symbol with a null
+    // storage class that this generates.
     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()) {
       if (Align <= 4) {
         COMDATSymName = "__real@" + scalarConstantToHexString(C);
         Align = 4;

test/CodeGen/X86/win_cst_pool.ll

 ; 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 -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"
 
 define double @double() {
   ret double 0x0000000000800000
 }
 ; CHECK:              .globl  __real@0000000000800000
 ; CHECK-NEXT:         .section        .rdata,"dr",discard,__real@0000000000800000
 ; CHECK-NEXT:         .p2align  3
 ; CHECK-NEXT: __real@0000000000800000:
 ; CHECK-NEXT:         .quad   8388608
 ; CHECK:      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> <i32 3, i32 2, i32 1, i32 0>
 }
 ; CHECK:              .globl  __xmm@00000000000000010000000200000003
 ; CHECK-NEXT:         .section        .rdata,"dr",discard,__xmm@00000000000000010000000200000003
 ; CHECK-NEXT:         .p2align  4
 ; CHECK-NEXT: __xmm@00000000000000010000000200000003:
 ; CHECK-NEXT:         .long   3