[TargetMachine] Don't try to create COFFSTUB references on windows on non-COFF

[mstorsjo]

Summary

This avoids spurious relocation types for windows/elf targets, as reported in D51590.

@hans, once settled, I think this is a bugfix important enough for 9.0 if it still can make it, as it currently is breaking Julia users.

[rnk]

lib/Target/TargetMachine.cpp

148 ↗

(On Diff #215800)

Maybe this should be relaxed to just isCOFF || isWindows, rather than enumerating other formats.

lib/Target/X86/X86Subtarget.cpp

149 ↗

(On Diff #215800)

This seems like it's trivially dead (Windows must use COFF, right? Actually, no), so I think it's worth a comment.

[mstorsjo]

lib/Target/TargetMachine.cpp

148 ↗

(On Diff #215800)

Yes, that's probably best.

[mstorsjo]

Simplified condition to (coff || windows), added a comment

[hans]

@hans, once settled, I think this is a bugfix important enough for 9.0 if it still can make it, as it currently is breaking Julia users.

Thanks, I've put it on my watchlist.

[vchuravy]

Thanks Martin for the quick response and fix. My minimal test-case works and I am in the progress of testing our full build.

[rnk]

lgtm

[mstorsjo]

This apparently broke tests for a few other architectures, when the testsuite is run on windows: http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/9816/steps/stage%201%20check/logs/stdio

These tests use plain e.g. llc -march=amdgcn. When run on windows, that ends up implicitly using a triple with a windows os, even though the tests probably didn't ever intend that.

I guess it should be ok to just amend those tests and change -march into -mtriple with a suitable non-windows os?

[mstorsjo]

This change should hopefully fix the errors in that log:

diff --git a/test/CodeGen/AMDGPU/propagate-attributes-bitcast-function.ll b/test/CodeGen/AMDGPU/propagate-attributes-bitcast-function.ll
index c1d8009d08b..173bc72db85 100644
--- a/test/CodeGen/AMDGPU/propagate-attributes-bitcast-function.ll
+++ b/test/CodeGen/AMDGPU/propagate-attributes-bitcast-function.ll
@@ -1,4 +1,4 @@
-; RUN: llc -march=amdgcn -mcpu=gfx1010 -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx1010 -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s
 
 ; GCN: foo1:
 ; v_cndmask_b32_e64 v0, 0, 1, vcc_lo{{$}}
diff --git a/test/CodeGen/AMDGPU/propagate-attributes-clone.ll b/test/CodeGen/AMDGPU/propagate-attributes-clone.ll
index b9c36217aaa..cb0406c5b1f 100644
--- a/test/CodeGen/AMDGPU/propagate-attributes-clone.ll
+++ b/test/CodeGen/AMDGPU/propagate-attributes-clone.ll
@@ -1,5 +1,5 @@
 ; RUN: opt -S -mtriple=amdgcn-amd-amdhsa -O1 < %s | FileCheck -check-prefix=OPT %s
-; RUN: llc -march=amdgcn -mcpu=gfx1010 -verify-machineinstrs < %s | FileCheck -check-prefix=LLC %s
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx1010 -verify-machineinstrs < %s | FileCheck -check-prefix=LLC %s
 
 ; OPT: declare void @foo4() local_unnamed_addr #0
 ; OPT: define internal fastcc void @foo3.2() unnamed_addr #1
diff --git a/test/CodeGen/AMDGPU/propagate-attributes-single-set.ll b/test/CodeGen/AMDGPU/propagate-attributes-single-set.ll
index 34882586533..cb4283c8c67 100644
--- a/test/CodeGen/AMDGPU/propagate-attributes-single-set.ll
+++ b/test/CodeGen/AMDGPU/propagate-attributes-single-set.ll
@@ -1,5 +1,5 @@
 ; RUN: opt -S -mtriple=amdgcn-amd-amdhsa -O1 < %s | FileCheck -check-prefix=OPT %s
-; RUN: llc -march=amdgcn -mcpu=gfx1010 -verify-machineinstrs < %s | FileCheck -check-prefix=LLC %s
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx1010 -verify-machineinstrs < %s | FileCheck -check-prefix=LLC %s
 
 ; OPT: declare void @foo4() local_unnamed_addr #0
 ; OPT: define void @foo3() local_unnamed_addr #1
diff --git a/test/CodeGen/Hexagon/pic-jt-big.ll b/test/CodeGen/Hexagon/pic-jt-big.ll
index f5b4c2df52c..25ee04521b6 100644
--- a/test/CodeGen/Hexagon/pic-jt-big.ll
+++ b/test/CodeGen/Hexagon/pic-jt-big.ll
@@ -1,4 +1,4 @@
-; RUN: llc -march=hexagon -relocation-model=pic < %s | FileCheck %s
+; RUN: llc -mtriple=hexagon-unknown-elf -relocation-model=pic < %s | FileCheck %s
 
 ; CHECK: r{{[0-9]+}} = add({{pc|PC}},##.LJTI{{[0-9_]+}}@PCREL)
 ; CHECK: r{{[0-9]+}} = memw(r{{[0-9]}}+##g0@GOT
diff --git a/test/CodeGen/Hexagon/pic-sdata.ll b/test/CodeGen/Hexagon/pic-sdata.ll
index 3e4dc2dc93e..446734b9ca0 100644
--- a/test/CodeGen/Hexagon/pic-sdata.ll
+++ b/test/CodeGen/Hexagon/pic-sdata.ll
@@ -1,5 +1,5 @@
-; RUN: llc -march=hexagon -hexagon-small-data-threshold=8 -relocation-model=static < %s | FileCheck --check-prefixes=CHECK,STATIC %s
-; RUN: llc -march=hexagon -hexagon-small-data-threshold=8 -relocation-model=pic < %s | FileCheck --check-prefixes=CHECK,PIC %s
+; RUN: llc -mtriple=hexagon-unknown-elf -hexagon-small-data-threshold=8 -relocation-model=static < %s | FileCheck --check-prefixes=CHECK,STATIC %s
+; RUN: llc -mtriple=hexagon-unknown-elf -hexagon-small-data-threshold=8 -relocation-model=pic < %s | FileCheck --check-prefixes=CHECK,PIC %s
 
 ; If a global has a specified section, it should probably be placed in that
 ; section, but with PIC any accesses to globals in small data should still
diff --git a/test/CodeGen/SPARC/tls.ll b/test/CodeGen/SPARC/tls.ll
index 1b1af2e99c3..843769df8d9 100644
--- a/test/CodeGen/SPARC/tls.ll
+++ b/test/CodeGen/SPARC/tls.ll
@@ -1,12 +1,12 @@
-; RUN: llc <%s -march=sparc   -relocation-model=static | FileCheck %s --check-prefix=v8abs
-; RUN: llc <%s -march=sparcv9 -relocation-model=static | FileCheck %s --check-prefix=v9abs
-; RUN: llc <%s -march=sparc   -relocation-model=pic    | FileCheck %s --check-prefix=pic
-; RUN: llc <%s -march=sparcv9 -relocation-model=pic    | FileCheck %s --check-prefix=pic
-
-; RUN: llc <%s -march=sparc   -relocation-model=static -filetype=obj | llvm-readobj -r --symbols | FileCheck %s --check-prefix=v8abs-obj
-; RUN: llc <%s -march=sparcv9 -relocation-model=static -filetype=obj | llvm-readobj -r --symbols | FileCheck %s --check-prefix=v9abs-obj
-; RUN: llc <%s -march=sparc   -relocation-model=pic    -filetype=obj | llvm-readobj -r --symbols | FileCheck %s --check-prefix=pic-obj
-; RUN: llc <%s -march=sparcv9 -relocation-model=pic    -filetype=obj | llvm-readobj -r --symbols | FileCheck %s --check-prefix=pic-obj
+; RUN: llc <%s -mtriple=sparc-unknown-linux   -relocation-model=static | FileCheck %s --check-prefix=v8abs
+; RUN: llc <%s -mtriple=sparcv9-unknown-linux -relocation-model=static | FileCheck %s --check-prefix=v9abs
+; RUN: llc <%s -mtriple=sparc-unknown-linux   -relocation-model=pic    | FileCheck %s --check-prefix=pic
+; RUN: llc <%s -mtriple=sparcv9-unknown-linux -relocation-model=pic    | FileCheck %s --check-prefix=pic
+
+; RUN: llc <%s -mtriple=sparc-unknown-linux   -relocation-model=static -filetype=obj | llvm-readobj -r --symbols | FileCheck %s --check-prefix=v8abs-obj
+; RUN: llc <%s -mtriple=sparcv9-unknown-linux -relocation-model=static -filetype=obj | llvm-readobj -r --symbols | FileCheck %s --check-prefix=v9abs-obj
+; RUN: llc <%s -mtriple=sparc-unknown-linux   -relocation-model=pic    -filetype=obj | llvm-readobj -r --symbols | FileCheck %s --check-prefix=pic-obj
+; RUN: llc <%s -mtriple=sparcv9-unknown-linux -relocation-model=pic    -filetype=obj | llvm-readobj -r --symbols | FileCheck %s --check-prefix=pic-obj
 
 @local_symbol = internal thread_local global i32 0
 @extern_symbol = external thread_local global i32

Does that sound sensible? I tried to pick triple values matching what's used elsewhere in similar tests.

[mstorsjo]

I landed my tweak for the tests in rL369443, let's hope that fixes the buildbot.

[hans]

In D66401#1636781, @hans wrote:

@hans, once settled, I think this is a bugfix important enough for 9.0 if it still can make it, as it currently is breaking Julia users.

Thanks, I've put it on my watchlist.

Merged to release_90 together with r369443 in r369654.

llvm/trunk/lib/Target/TargetMachine.cpp

bool TargetMachine::shouldAssumeDSOLocal(const Module &M,
   if (GV && GV->hasDLLImportStorageClass())
     return false;
 
   // On MinGW, variables that haven't been declared with DLLImport may still
   // end up automatically imported by the linker. To make this feasible,
   // don't assume the variables to be DSO local unless we actually know
   // that for sure. This only has to be done for variables; for functions
   // the linker can insert thunks for calling functions from another DLL.
-  if (TT.isWindowsGNUEnvironment() && GV && GV->isDeclarationForLinker() &&
-      isa<GlobalVariable>(GV))
+  if (TT.isWindowsGNUEnvironment() && TT.isOSBinFormatCOFF() && GV &&
+      GV->isDeclarationForLinker() && isa<GlobalVariable>(GV))
     return false;
 
   // On COFF, don't mark 'extern_weak' symbols as DSO local. If these symbols
   // remain unresolved in the link, they can be resolved to zero, which is
   // outside the current DSO.
   if (TT.isOSBinFormatCOFF() && GV && GV->hasExternalWeakLinkage())
     return false;
 
   // Every other GV is local on COFF.
   // Make an exception for windows OS in the triple: Some firmware builds use
   // *-win32-macho triples. This (accidentally?) produced windows relocations
   // without GOT tables in older clang versions; Keep this behaviour.
-  if (TT.isOSBinFormatCOFF() || (TT.isOSWindows() && TT.isOSBinFormatMachO()))
+  // Some JIT users use *-win32-elf triples; these shouldn't use GOT tables
+  // either.
+  if (TT.isOSBinFormatCOFF() || TT.isOSWindows())
     return true;
 
   // Most PIC code sequences that assume that a symbol is local cannot
   // produce a 0 if it turns out the symbol is undefined. While this
   // is ABI and relocation depended, it seems worth it to handle it
   // here.
   if (GV && isPositionIndependent() && GV->hasExternalWeakLinkage())
     return false;

llvm/trunk/lib/Target/X86/X86Subtarget.cpp

unsigned char X86Subtarget::classifyGlobalReference(const GlobalValue *GV,
   if (TM.shouldAssumeDSOLocal(M, GV))
     return classifyLocalReference(GV);
 
   if (isTargetCOFF()) {
     if (GV->hasDLLImportStorageClass())
       return X86II::MO_DLLIMPORT;
     return X86II::MO_COFFSTUB;
   }
+  // Some JIT users use *-win32-elf triples; these shouldn't use GOT tables.
+  if (isOSWindows())
+    return X86II::MO_NO_FLAG;
 
   if (is64Bit()) {
     // ELF supports a large, truly PIC code model with non-PC relative GOT
     // references. Other object file formats do not. Use the no-flag, 64-bit
     // reference for them.
     if (TM.getCodeModel() == CodeModel::Large)
       return isTargetELF() ? X86II::MO_GOT : X86II::MO_NO_FLAG;
     return X86II::MO_GOTPCREL;

llvm/trunk/test/CodeGen/X86/mingw-refptr.ll

 ; RUN: llc < %s -mtriple=x86_64-w64-mingw32 | FileCheck %s -check-prefix=CHECK-X64
 ; RUN: llc < %s -mtriple=i686-w64-mingw32 | FileCheck %s -check-prefix=CHECK-X86
+; RUN: llc < %s -mtriple=i686-w64-mingw32-none-elf | FileCheck %s -check-prefix=CHECK-X86-ELF
 
 @var = external local_unnamed_addr global i32, align 4
 @dsolocalvar = external dso_local local_unnamed_addr global i32, align 4
 @localvar = dso_local local_unnamed_addr global i32 0, align 4
 @localcommon = common dso_local local_unnamed_addr global i32 0, align 4
 @extvar = external dllimport local_unnamed_addr global i32, align 4
 
 define dso_local i32 @getVar() {
 ; CHECK-X64-LABEL: getVar:
 ; CHECK-X64:    movq .refptr.var(%rip), %rax
 ; CHECK-X64:    movl (%rax), %eax
 ; CHECK-X64:    retq
 ; CHECK-X86-LABEL: _getVar:
 ; CHECK-X86:    movl .refptr._var, %eax
 ; CHECK-X86:    movl (%eax), %eax
 ; CHECK-X86:    retl
+; CHECK-X86-ELF-LABEL: getVar:
+; CHECK-X86-ELF:    movl var, %eax
+; CHECK-X86-ELF:    retl
 entry:
   %0 = load i32, i32* @var, align 4
   ret i32 %0
 }
 
 define dso_local i32 @getDsoLocalVar() {
 ; CHECK-X64-LABEL: getDsoLocalVar:
 ; CHECK-X64:    movl dsolocalvar(%rip), %eax

 ; CHECK-X64-LABEL: getExtVar:
 ; CHECK-X64:    movq __imp_extvar(%rip), %rax
 ; CHECK-X64:    movl (%rax), %eax
 ; CHECK-X64:    retq
 ; CHECK-X86-LABEL: _getExtVar:
 ; CHECK-X86:    movl __imp__extvar, %eax
 ; CHECK-X86:    movl (%eax), %eax
 ; CHECK-X86:    retl
+; CHECK-X86-ELF-LABEL: getExtVar:
+; CHECK-X86-ELF:    movl extvar, %eax
+; CHECK-X86-ELF:    retl
 entry:
   %0 = load i32, i32* @extvar, align 4
   ret i32 %0
 }
 
 define dso_local void @callFunc() {
 ; CHECK-X64-LABEL: callFunc:
 ; CHECK-X64:    jmp otherFunc