[PATCH] D43514: Start settinng dso_local for COFF

[Eric Christopher via cfe-commits]

On Thu, Feb 22, 2018 at 4:40 PM Rafael Avila de Espindola via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Eric Christopher via Phabricator <reviews at reviews.llvm.org> writes:
>
> > echristo added inline comments.
> >
> >
> > ================
> > Comment at: lib/CodeGen/CodeGenModule.h:728
> > +  /// This must be called after dllimport/dllexport is set.
> > +  /// FIXME: should this set dllimport/dllexport instead?
> >    void setGVProperties(llvm::GlobalValue *GV, const NamedDecl *D) const;
> > ----------------
> > Agreed? Maybe? Should the rest of the properties from above each call be
> set in this function?
> >
> > Since you've got the decl it might be best to get as many of them in one
> place as possible?
>
> Very likely yes.
>
> One think I have locally is a very ugly patch that sets dso_local all
> over the place in clang until every GV in coff is dso_local or
> dllimport.
>
> My idea is to cleanup it bit by bit once the initial patch (this one) is
> in.
>
> I can try moving dllimport/dllexport setting to setGVProperties as the
> first followup patch if you agree.
>
>
SGTM. I'll ack the patch.

-eric
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180223/0cb1bdb7/attachment.html>