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
