<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Thu, Feb 22, 2018 at 4:40 PM Rafael Avila de Espindola via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Eric Christopher via Phabricator <<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>> writes:<br>
<br>
> echristo added inline comments.<br>
><br>
><br>
> ================<br>
> Comment at: lib/CodeGen/CodeGenModule.h:728<br>
> +  /// This must be called after dllimport/dllexport is set.<br>
> +  /// FIXME: should this set dllimport/dllexport instead?<br>
>    void setGVProperties(llvm::GlobalValue *GV, const NamedDecl *D) const;<br>
> ----------------<br>
> Agreed? Maybe? Should the rest of the properties from above each call be set in this function?<br>
><br>
> Since you've got the decl it might be best to get as many of them in one place as possible?<br>
<br>
Very likely yes.<br>
<br>
One think I have locally is a very ugly patch that sets dso_local all<br>
over the place in clang until every GV in coff is dso_local or<br>
dllimport.<br>
<br>
My idea is to cleanup it bit by bit once the initial patch (this one) is<br>
in.<br>
<br>
I can try moving dllimport/dllexport setting to setGVProperties as the<br>
first followup patch if you agree.<br><br></blockquote><div><br></div><div>SGTM. I'll ack the patch.</div><div><br></div><div>-eric </div></div></div>