[Patch] (take 2) New EliminateAvailableExternally pass / Pass down -flto

[Teresa Johnson]

And I'm out on vacation again this week. :). Thanks, will commit when I am
back Mon Jul 5.
Teresa

On Mon, Jun 29, 2015, 5:04 PM Duncan P. N. Exon Smith <dexonsmith at apple.com>
wrote:

> Whereas I just missed this go by and have no excuse :/.  LGTM too, FWIW.
>
> > On 2015-Jun-29, at 16:54, Reid Kleckner <rnk at google.com> wrote:
> >
> > Back from vacation myself. It's that time of year. :)
> >
> > lgtm
> >
> > On Tue, Jun 23, 2015 at 11:39 AM, Teresa Johnson <tejohnson at google.com>
> wrote:
> >
> >
> > On Thu, Jun 11, 2015 at 8:17 PM, Teresa Johnson <tejohnson at google.com>
> wrote:
> > Hi Nico,
> >
> > Sorry about that. Since I am heading out on vacation for a week tomorrow
> I went ahead and reverted for now.
> >
> > Teresa
> >
> > On Thu, Jun 11, 2015 at 6:07 PM, Nico Weber <thakis at chromium.org> wrote:
> > Hi Teresa,
> >
> > this (well, 239480 really) seems to break building dynamic libraries
> pretty decisively:
> https://code.google.com/p/chromium/issues/detail?id=499508#c3 Can you
> take a look, and if it takes a while to investigate, revert this for now?
> >
> > Thanks,
> > Nico
> >
> > I am back from vacation and found what was happening here. The attached
> patches are largely the same as the original ones but contain a fix for
> this issue (llvm patch) and a new test created from Nico's reduced test
> (clang patch).
> >
> > The broken code was compiled -fvisibility=hidden, and this caused the
> thunk to SyncMessageFilter::Send
> (_ZThn16_N17SyncMessageFilter4SendEP7Message), which is
> available_externally, to also be marked hidden.
> >
> > With my patch, we eliminated the function's body and turned it into a
> declaration, which was still marked hidden as I wasn't modifying
> visibility. During AsmPrinter::doFinalization, EmitVisibility is called on
> all function declarations in the module, which caused this symbol to get
> hidden visibility (via a resulting call to MCSymbolELF::setVisibility). The
> linker then complained because it was undefined and hidden.
> >
> > Before my patch, code gen suppressed the emission of this function since
> it was available externally, and as a result, EmitVisibility, and thus
> MCSymbolELF::setVisibility, were simply never called. The undefined symbol
> then ended up with the default visibility. It seems to me that this
> essentially worked by luck.
> >
> > I've fixed this by changing the visibility on globals to
> DefaultVisibility when we eliminate their definitions in the new
> EliminateAvailableExternally pass.
> >
> > Patches attached. Tests (including the new one) all pass. Ok to commit?
> >
> > Thanks,
> > Teresa
> >
> >
> > On Wed, Jun 10, 2015 at 10:49 AM, Teresa Johnson <tejohnson at google.com>
> wrote:
> > Author: tejohnson
> > Date: Wed Jun 10 12:49:45 2015
> > New Revision: 239481
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=239481&view=rev
> > Log:
> > Pass down the -flto option to the -cc1 job, and from there into the
> > CodeGenOptions and onto the PassManagerBuilder. This enables gating
> > the new EliminateAvailableExternally module pass on whether we are
> > preparing for LTO.
> >
> > If we are preparing for LTO (e.g. a -flto -c compile), the new pass is
> not
> > included as we want to preserve available externally functions for
> possible
> > link time inlining.
> >
> > --
> > Teresa Johnson |       Software Engineer |     tejohnson at google.com |
> 408-460-2413
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150630/fad4a5e0/attachment.html>