[TargetMachine] Simplify shouldAssumeDSOLocal by processing ExternalSymbolSDNode early

The function accrues many `GV` nullness checks. Process `!GV`
(ExternalSymbolSDNode) early to simplify code.

Also improve a comment added in r327198 (intrinsics is a subset of
ExternalSymbolSDNode).

Intended to be NFC.

llvm/lib/Target/TargetMachine.cpp

@@ -93,21 +93,15 @@ static TLSModel::Model getSelectedTLSModel(const GlobalValue *GV) {
 
 bool TargetMachine::shouldAssumeDSOLocal(const Module &M,
                                          const GlobalValue *GV) const {
-  // If the IR producer requested that this GV be treated as dso local, obey.
-  if (GV && GV->isDSOLocal())
-    return true;
-
-  // If we are not supossed to use a PLT, we cannot assume that intrinsics are
-  // local since the linker can convert some direct access to access via plt.
-  if (M.getRtLibUseGOT() && !GV)
-    return false;
+  const Triple &TT = getTargetTriple();
+  Reloc::Model RM = getRelocationModel();
 
   // According to the llvm language reference, we should be able to
   // just return false in here if we have a GV, as we know it is
   // 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 intrinsics, GV is null and there is nowhere to put
+  // In the case of ExternalSymbolSDNode, GV is null and there is nowhere to put
   // dso_local. Returning false for those will produce worse code in some
   // architectures. For example, on x86 the caller has to set ebx before calling
   // a plt.
@@ -115,12 +109,24 @@ bool TargetMachine::shouldAssumeDSOLocal(const Module &M,
   // generated code.
   // FIXME: Add a module level metadata for whether intrinsics should be assumed
   // local.
+  if (!GV) {
+    if (TT.isOSBinFormatCOFF())
+      return true;
+    if (TT.isOSBinFormatELF() && TT.isX86() && RM == Reloc::Static) {
+      // For -fno-plt, we cannot assume that intrinsics are local since the
+      // linker can convert some direct access to access via plt.
+      return !M.getRtLibUseGOT();
+    }
 
-  Reloc::Model RM = getRelocationModel();
-  const Triple &TT = getTargetTriple();
+    return false;
+  }
+
+  // If the IR producer requested that this GV be treated as dso local, obey.
+  if (GV->isDSOLocal())
+    return true;
 
   // DLLImport explicitly marks the GV as external.
-  if (GV && GV->hasDLLImportStorageClass())
+  if (GV->hasDLLImportStorageClass())
     return false;
 
   // On MinGW, variables that haven't been declared with DLLImport may still
@@ -128,14 +134,14 @@ bool TargetMachine::shouldAssumeDSOLocal(const Module &M,
   // 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() && TT.isOSBinFormatCOFF() && GV &&
+  if (TT.isWindowsGNUEnvironment() && TT.isOSBinFormatCOFF() &&
       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())
+  if (TT.isOSBinFormatCOFF() && GV->hasExternalWeakLinkage())
     return false;
 
   // Every other GV is local on COFF.
@@ -151,16 +157,16 @@ bool TargetMachine::shouldAssumeDSOLocal(const Module &M,
   // 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())
+  if (isPositionIndependent() && GV->hasExternalWeakLinkage())
     return false;
 
-  if (GV && !GV->hasDefaultVisibility())
+  if (!GV->hasDefaultVisibility())
     return true;
 
   if (TT.isOSBinFormatMachO()) {
     if (RM == Reloc::Static)
       return true;
-    return GV && GV->isStrongDefinitionForLinker();
+    return GV->isStrongDefinitionForLinker();
   }
 
   // Due to the AIX linkage model, any global with default visibility is
@@ -175,13 +181,13 @@ bool TargetMachine::shouldAssumeDSOLocal(const Module &M,
       RM == Reloc::Static || M.getPIELevel() != PIELevel::Default;
   if (IsExecutable) {
     // If the symbol is defined, it cannot be preempted.
-    if (GV && !GV->isDeclarationForLinker())
+    if (!GV->isDeclarationForLinker())
       return true;
 
     // A symbol marked nonlazybind should not be accessed with a plt. If the
     // symbol turns out to be external, the linker will convert a direct
     // access to an access via the plt, so don't assume it is local.
-    const Function *F = dyn_cast_or_null<Function>(GV);
+    const Function *F = dyn_cast<Function>(GV);
     if (F && F->hasFnAttribute(Attribute::NonLazyBind))
       return false;
     Triple::ArchType Arch = TT.getArch();
@@ -196,7 +202,7 @@ bool TargetMachine::shouldAssumeDSOLocal(const Module &M,
     if (RM == Reloc::Static) {
       // We currently respect dso_local/dso_preemptable specifiers for
       // variables.
-      if (!GV || F)
+      if (F)
         return true;
       // TODO Remove the special case for x86-32.
       if (Arch == Triple::x86 && !GV->isThreadLocal())
@@ -207,7 +213,7 @@ bool TargetMachine::shouldAssumeDSOLocal(const Module &M,
     // alias, set the flag. We cannot set dso_local for other global values,
     // because otherwise direct accesses to a probably interposable symbol (even
     // if the codegen assumes not) will be rejected by the linker.
-    if (!GV || !GV->canBenefitFromLocalAlias())
+    if (!GV->canBenefitFromLocalAlias())
       return false;
     return TT.isX86() && M.noSemanticInterposition();
   }