[PATCH] D19102: Always traverse GlobalVariable initializer when computing the export list
Mehdi AMINI via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 22 14:50:29 PDT 2016
joker.eph added inline comments.
================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:173
@@ +172,3 @@
+ // the initializer for globals that are referenced. We mimic this behavior
+ // here so that the ExportList is accurate.
+ for (auto &Ref : GVS->refs()) {
----------------
tejohnson wrote:
> Can you update the comment which is inaccurate since we don't always import initializers? Oh, I just figured out why this is getting imported. It is related to #3 from my earlier list of when materializeInitFor will decide to linkGlobalValueBody():
>
> > 3. New hasLocalLinkage(): We should have promoted any locals referenced by an import before this, so shouldn't be true here
>
> But I was wrong! The one situation where we will not promote a local is a global variable that is constant with unnamed addr:
>
> bool FunctionImportGlobalProcessing::doPromoteLocalToGlobal(
> ...
> auto *GVar = dyn_cast<GlobalVariable>(SGV);
> if (GVar && GVar->isConstant() && GVar->hasUnnamedAddr())
> return false;
>
> which is exactly what happens in your test case. Can you update the comment accordingly?
>
> Also, maybe add a FIXME that if we had a flag in summary indicating that GVar was constant that we could refine this?
I'll update.
(added a note in our TODO shared doc as well)
http://reviews.llvm.org/D19102
More information about the llvm-commits
mailing list