What to do with LTO_SYMBOL_SCOPE_DEFAULT_CAN_BE_HIDDEN?

[Nick Lewycky]

On 31 October 2013 08:35, Rafael Espíndola <rafael.espindola at gmail.com>wrote:

> > Lots of deletes! That makes me smile.
>
> Same here :-)
>
> > Also, removing a C API call. That's never been in a release, right?
>
> Correct. The feature was first implemented with an API close to what
> the gold plugin implements, but then I figure out I could use the
> existing LTO_SYMBOL_SCOPE_DEFAULT_CAN_BE_HIDDEN by moving a bit more
> work to the plugin. Fortunately the API has not been exposed in any
> release yet.
>
> > @@ -162,6 +163,8 @@ LTOModule *LTOModule::makeLTOModule(MemoryBuffer
> > *buffer,
> >
> >    TargetMachine *target = march->createTargetMachine(TripleStr, CPU,
> > FeatureStr,
> >                                                       options);
> > +  m->MaterializeAllPermanently();
> > +
> >    LTOModule *Ret = new LTOModule(m.take(), target);
> >    if (Ret->parseSymbols(errMsg)) {
> >      delete Ret;
> >
> > Hah! You should talk to that guy who made LTO work with lazily loaded
> > bitcode. ;-)
>
> Yes, this is the trade-off. I can be fixed with the address-taken bit
> proposal for example. For now the benefit of not having the extra C
> api in 3.4 seems worth it.
>
> > +// For example: Internalizing a function with external linkage. Only if
> we
> > are
> > +// told it is only used from within this module, it is safe to do it.
> >
> > "it is" -> "is it".
>
> Where? Both occurrences look correct to me? (but I am not too sure
> about the comma).


Reading aloud "only if we are told [...] is it safe to do it" sounds much
more natural to me, but I'll agree it's correct either way. Certainly not a
patch blocker!


>  > LGTM from me assuming that function has never been in a release before,
> but
> > I'd also like to see OtherNick sign off.
>
> Thanks!
>
> Nick Kledzik, is this OK with you?
>
> > Nick
>
> Cheers,
> Rafael
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131031/eae1b302/attachment.html>