[DwarfCompileUnit] getOrCreateCommonBlock(): check for existing entity first. NFCI

[krisb]

Summary

For global variables and common blocks there is no way to create entities
through getOrCreateContextDIE(), so no need to obtain the context first.

[dblaikie]

Ah, looks like the comment was never accurate (& was copy/pasted from various other handling, which is more likely to be accurate in the other places (like types, rather than variables)) when introduced in f6b936bc062e7490cfe956d58d47ba92b46e830f

Mixed feelings about possible implications of making a (albeit NFCI) code change to match a comment rather than changing the comment to match the code. But to make this case consistent with the other cases the comment was copy/pasted from, sure let's go with this

[krisb]

Make getOrCreateCommonBlock() to check for existing entity first, remove outdated comments.

[krisb]

Mixed feelings about possible implications of making a (albeit NFCI) code change to match a comment rather than changing the comment to match the code. But to make this case consistent with the other cases the comment was copy/pasted from, sure let's go with this

Heh, this didn't seem harmful to me by the first look, but now I'm starting thinking that we may get a lot of additional map queries if the parent-child chain (for the context) is long enough.
I guess it is not possible to create a global variable through getOrCreateContextDIE() atm, the same is for fortran's common blocks. So, let's make an exception from the pattern for them.

[dblaikie]

I'd probably separate these two commits - one's updating a comment to reflect the reality of the code, the other is changing unrelated code (which, while NFCI, is a change & would be easier to track if it breaks buildbots, etc, if it's separate)

feel free to commit the global variable comment change without sending a separate review/etc

As for the common block one - yeah, that sounds OK if it's a reasonable invariant that the scope could never create the entity being created here.

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

return Asm->OutStreamer->emitDwarfFileDirective(0, "", "", None, None,
                                                     CUID);
   return Asm->OutStreamer->emitDwarfFileDirective(
       0, File->getDirectory(), File->getFilename(), DD->getMD5AsBytes(File),
       File->getSource(), CUID);
 }
 
 DIE *DwarfCompileUnit::getOrCreateGlobalVariableDIE(
     const DIGlobalVariable *GV, ArrayRef<GlobalExpr> GlobalExprs) {
-  // Check for pre-existence.
-  if (DIE *Die = getDIE(GV))
-    return Die;
-
   assert(GV);
 
   auto *GVContext = GV->getScope();
   const DIType *GTy = GV->getType();
 
   // Construct the context before querying for the existence of the DIE in
   // case such construction creates the DIE.
   auto *CB = GVContext ? dyn_cast<DICommonBlock>(GVContext) : nullptr;
   DIE *ContextDIE = CB ? getOrCreateCommonBlock(CB, GlobalExprs)
     : getOrCreateContextDIE(GVContext);
 
+  // Check for pre-existence.
+  if (DIE *Die = getDIE(GV))
+    return Die;
+
   // Add to map.
   DIE *VariableDIE = &createAndAddDIE(GV->getTag(), *ContextDIE, GV);
   DIScope *DeclContext;
   if (auto *SDMDecl = GV->getStaticDataMemberDeclaration()) {
     DeclContext = SDMDecl->getScope();
     assert(SDMDecl->isStaticMember() && "Expected static member decl");
     assert(GV->isDefinition());
     // We need the declaration DIE that is in the static member's class.