[llvm] ba6e15d - [TargetMachine] Move COFF special case for ExternalSymbolSDNode from shouldAssumeDSOLocal to X86Subtarget

[Fangrui Song via llvm-commits]

Author: Fangrui Song
Date: 2021-08-23T13:54:40-07:00
New Revision: ba6e15d8cc52fab35dde41ab34b43283554c9a59

URL: https://github.com/llvm/llvm-project/commit/ba6e15d8cc52fab35dde41ab34b43283554c9a59
DIFF: https://github.com/llvm/llvm-project/commit/ba6e15d8cc52fab35dde41ab34b43283554c9a59.diff

LOG: [TargetMachine] Move COFF special case for ExternalSymbolSDNode from shouldAssumeDSOLocal to X86Subtarget

Intended to be NFC. ARM/AArch64 don't appear to need adjustment.

TargetMachine::shouldAssumeDSOLocal is expected to be very simple, ideally
matching isDSOLocal(). The IR producers are expected to set dso_local correctly.
(While some may think this function can make producers' work easier, the
function is really not in a good position to set dso_local. See the various
special cases we duplicate from clang CodeGenModule.cpp.)

Reviewed By: mstorsjo

Differential Revision: https://reviews.llvm.org/D108514

Added: 
    

Modified: 
    llvm/lib/Target/TargetMachine.cpp
    llvm/lib/Target/X86/X86Subtarget.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/TargetMachine.cpp b/llvm/lib/Target/TargetMachine.cpp
index 32bdb71b45328..08295df376e13 100644
--- a/llvm/lib/Target/TargetMachine.cpp
+++ b/llvm/lib/Target/TargetMachine.cpp
@@ -101,15 +101,11 @@ bool TargetMachine::shouldAssumeDSOLocal(const Module &M,
   // dso_preemptable.  At this point in time, the various IR producers
   // have not been transitioned to always produce a dso_local when it
   // is possible to do so.
-  // In the case of ExternalSymbolSDNode, GV is null and we should just return
-  // false. However, COFF currently relies on this to be true
   //
   // As a result we still have some logic in here to improve the quality of the
   // generated code.
-  // FIXME: Add a module level metadata for whether intrinsics should be assumed
-  // local.
   if (!GV)
-    return TT.isOSBinFormatCOFF();
+    return false;
 
   // If the IR producer requested that this GV be treated as dso local, obey.
   if (GV->isDSOLocal())

diff  --git a/llvm/lib/Target/X86/X86Subtarget.cpp b/llvm/lib/Target/X86/X86Subtarget.cpp
index 4af0ac238f598..b985a3aa52102 100644
--- a/llvm/lib/Target/X86/X86Subtarget.cpp
+++ b/llvm/lib/Target/X86/X86Subtarget.cpp
@@ -143,6 +143,9 @@ unsigned char X86Subtarget::classifyGlobalReference(const GlobalValue *GV,
     return classifyLocalReference(GV);
 
   if (isTargetCOFF()) {
+    // ExternalSymbolSDNode like _tls_index.
+    if (!GV)
+      return X86II::MO_NO_FLAG;
     if (GV->hasDLLImportStorageClass())
       return X86II::MO_DLLIMPORT;
     return X86II::MO_COFFSTUB;
@@ -184,10 +187,13 @@ X86Subtarget::classifyGlobalFunctionReference(const GlobalValue *GV,
   if (TM.shouldAssumeDSOLocal(M, GV))
     return X86II::MO_NO_FLAG;
 
-  // Functions on COFF can be non-DSO local for two reasons:
+  // Functions on COFF can be non-DSO local for three reasons:
+  // - They are intrinsic functions (!GV)
   // - They are marked dllimport
   // - They are extern_weak, and a stub is needed
   if (isTargetCOFF()) {
+    if (!GV)
+      return X86II::MO_NO_FLAG;
     if (GV->hasDLLImportStorageClass())
       return X86II::MO_DLLIMPORT;
     return X86II::MO_COFFSTUB;