erichkeane added inline comments.
================ Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6390-6391 llvm::GlobalValue *Val = I.second; - if (Val && !getModule().getNamedValue(Name->getName())) + llvm::GlobalValue *ExistingElem = + getModule().getNamedValue(Name->getName()); + ---------------- tahonermann wrote: > I suggest moving the declaration of `ExistingElem` after the break on `!Val` > given that the former is not relevant if `Val` is null. > > I think the `!Val` check is worth a comment here. I suggest: > // If Val is null, that implies that there were multiple declarations that > each > // had a claim to the unmangled name. In this case, generation of the alias > // is suppressed. See CodeGenModule::MaybeHandleStaticInExternC(). > > The lack of checking for an existing element with the desired alias name was > a pre-existing oversight, yes? SGTM, thanks! ================ Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6396-6397 + + // Simple case, where there are no ifuncs involved. + if (!ExistingElem || CheckAndReplaceExternCIFuncs(ExistingElem, Val)) addCompilerUsedGlobal(llvm::GlobalAlias::create(Name->getName(), Val)); ---------------- tahonermann wrote: > This comment seems inconsistent with the code. If there are no ifuncs > involved, then why call `CheckAndReplaceExternCIFuncs()`? Woops! Artifact of a previous implementation of this. Replaced the comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122608/new/ https://reviews.llvm.org/D122608 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits