IR: Disable verifier check for GlobalValues with private linkage named after a comdat for non-COFF.

[pcc]

Summary

This check is only meaningful for COFF and it is perfectly valid to create
such a GlobalValue in ELF.

[pcc]

• Remove decl

[rnk]

I haven't had time to really think about this, but I believe @majnemer felt it was important to add verifier checks like these. There's no reason we can't make this triple dependent, right? If there's no triple on the module, we can skip the check.

[pcc]

Making it conditional on the triple is fine with me I suppose. It's unfortunate that we use the same IR for ELF and COFF here, maybe longer term we should move towards exclusively using !associated in COFF.

[pcc]

• Check the triple

[rnk]

In the past, Rafael advocated for having more verifier check more things that are unrepresentable at an object file level. That's how we ended up with most of the checks in Verifier::visitGlobalAlias. I think this is consistent with that, even if we have to check which object file invariant we're enforcing.

lgtm

llvm/lib/IR/Verifier.cpp

 using namespace llvm;
 
 namespace llvm {
 
 struct VerifierSupport {
   raw_ostream *OS;
   const Module &M;
   ModuleSlotTracker MST;
+  Triple TT;
   const DataLayout &DL;
   LLVMContext &Context;
 
   /// Track the brokenness of the module while recursively visiting.
   bool Broken = false;
   /// Broken debug info can be "recovered" from by stripping the debug info.
   bool BrokenDebugInfo = false;
   /// Whether to treat broken debug info as an error.
   bool TreatBrokenDebugInfoAsError = true;
 
   explicit VerifierSupport(raw_ostream *OS, const Module &M)
-      : OS(OS), M(M), MST(&M), DL(M.getDataLayout()), Context(M.getContext()) {}
+      : OS(OS), M(M), MST(&M), TT(M.getTargetTriple()), DL(M.getDataLayout()),
+        Context(M.getContext()) {}
 
 private:
   void Write(const Module *M) {
     *OS << "; ModuleID = '" << M->getModuleIdentifier() << "'\n";
   }
 
   void Write(const Value *V) {
     if (V)
AssertDI(N.getTag() == dwarf::DW_TAG_imported_module ||
            "invalid tag", &N);
   if (auto *S = N.getRawScope())
     AssertDI(isa<DIScope>(S), "invalid scope for imported entity", &N, S);
   AssertDI(isDINode(N.getRawEntity()), "invalid imported entity", &N,
            N.getRawEntity());
 }
 
 void Verifier::visitComdat(const Comdat &C) {
-  // The Module is invalid if the GlobalValue has private linkage.  Entities
-  // with private linkage don't have entries in the symbol table.
+  // In COFF the Module is invalid if the GlobalValue has private linkage.
+  // Entities with private linkage don't have entries in the symbol table.
+  if (TT.isOSBinFormatCOFF())
     if (const GlobalValue *GV = M.getNamedValue(C.getName()))
-    Assert(!GV->hasPrivateLinkage(), "comdat global value has private linkage",
-           GV);
+      Assert(!GV->hasPrivateLinkage(),
+             "comdat global value has private linkage", GV);
 }
 
 void Verifier::visitModuleIdents(const Module &M) {
   const NamedMDNode *Idents = M.getNamedMetadata("llvm.ident");
   if (!Idents)
     return;
 
   // llvm.ident takes a list of metadata entry. Each entry has only one string.

llvm/test/Verifier/comdat2.ll

-; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: llvm-as %s -o /dev/null
+; RUN: opt -mtriple=x86_64-unknown-linux -o /dev/null
+; RUN: not opt -mtriple=x86_64-pc-win32 %s -o /dev/null 2>&1 | FileCheck %s
 
 $v = comdat any
 @v = private global i32 0, comdat($v)
 ; CHECK: comdat global value has private linkage