similar to: ThinLTO and linkonce_odr + unnamed_addr

There should be no semantic difference between linkonce_odr and weak_odr, except that weak_odr is non-discardable. Why doesn't the autohide optimization work just as well on weak_odr + unnamed_addr as linkonce_odr + unnamed_addr? On Tue, Feb 6, 2018 at 5:35 PM, Steven Wu via llvm-dev < llvm-dev at lists.llvm.org> wrote: > Hi, > > I recently found that thinLTO doesn't deal

I agree with Teresa, we should probably do #2 to preserve behavior for now. On Wed, Feb 7, 2018 at 9:34 AM, Teresa Johnson via llvm-dev < llvm-dev at lists.llvm.org> wrote: > Hi Steven, > > I'd prefer not to inhibit importing. I am also concerned about putting > these symbols in the llvm.compiler_used (I don't recall earlier discussion > around this, but it seems

Hi Steven, I'd prefer not to inhibit importing. I am also concerned about putting these symbols in the llvm.compiler_used (I don't recall earlier discussion around this, but it seems like it could have effects on optimization as you mention). What are the downsides of #2 (adding visibility hidden)? We already do this when promoting internal linkage to external due to importing. I'm

That is a good question and I don't know. The optimization is defined include/llvm/Analysis/ObjectUtils.h. If I enable that for weak_odr + unnamed_addr, no tests are failing so I guess it is a safe optimization? :) It is probably because the autohide optimization is targeted at c++ templates and inline functions and we know they have linkonce_odr linkage, which suggests whoever uses this

Something I haven't seen mentioned: why are we dropping the unnamed_addr? Can't we preserve it with the weak symbol? Would it be OK to add auto-hide in this case (maybe it would already happen)? Can we use the new local_unnamed_addr that was added (by pcc or Rafael I don't remember)? I think this attribute matches exactly the `auto-hide` semantic. Wasn't the idea that this could be

> But it is interesting in general because according to the definition for local_unnamed_addr, the symbol has to be linkonce_odr to be auto hide as well. ThinLTO promotion can break that as well. What do you mean that the promotion can break that? The original description I found here: https://reviews.llvm.org/D20348 says that it is possible to exclude a global from the symbol table if three

2018-02-08 9:33 GMT-08:00 Steven Wu <stevenwu at apple.com>: > > > On Feb 7, 2018, at 4:03 PM, Mehdi AMINI <joker.eph at gmail.com> wrote: > > > > 2018-02-07 12:45 GMT-08:00 Steven Wu <stevenwu at apple.com>: > >> >> >> On Feb 7, 2018, at 12:36 PM, Mehdi AMINI <joker.eph at gmail.com> wrote: >> >> > But it is

2018-02-07 12:45 GMT-08:00 Steven Wu <stevenwu at apple.com>: > > > On Feb 7, 2018, at 12:36 PM, Mehdi AMINI <joker.eph at gmail.com> wrote: > > > But it is interesting in general because according to the definition > for local_unnamed_addr, the symbol has to be linkonce_odr to be auto hide > as well. ThinLTO promotion can break that as well. > > What do

>From looking at the code, it seems like LLVM is basically opting MachO into -fvisibility-inlines-hidden all the time, i.e. if the function is linkonce, it's discardable, so mark it hidden to pretend the compiler inlined it and discarded it. However, this isn't conforming, because the addresses of inline functions will no longer compare equal across DSOs. Realistically, nobody cares

I didn't realize that that WeakDefCanBeHiddenDirective is only available on Darwin. So if we are doing it for #2, it should be a Darwin only fix as well. Steven > On Feb 7, 2018, at 11:29 AM, Reid Kleckner <rnk at google.com> wrote: > > I agree with Teresa, we should probably do #2 to preserve behavior for now. > > On Wed, Feb 7, 2018 at 9:34 AM, Teresa Johnson via

> On Feb 8, 2018, at 10:28 AM, Mehdi AMINI <joker.eph at gmail.com> wrote: > > > > 2018-02-08 9:33 GMT-08:00 Steven Wu <stevenwu at apple.com <mailto:stevenwu at apple.com>>: > > >> On Feb 7, 2018, at 4:03 PM, Mehdi AMINI <joker.eph at gmail.com <mailto:joker.eph at gmail.com>> wrote: >> >> >> >> 2018-02-07

We didn't drop unnamed_addr. I just don't think weakodr_addr + unnamed_addr is safe to hide in all cases. I don't know if I interpreted local_unnamed_addr correctly but I think it is mostly the same in thinLTO for ld64. The code generator only looks at the individual module and ld64 will be in charge of merging all the symbols with autohide. It doesn't really help in this case.

2018-02-08 10:44 GMT-08:00 Steven Wu <stevenwu at apple.com>: > > > On Feb 8, 2018, at 10:28 AM, Mehdi AMINI <joker.eph at gmail.com> wrote: > > > > 2018-02-08 9:33 GMT-08:00 Steven Wu <stevenwu at apple.com>: > >> >> >> On Feb 7, 2018, at 4:03 PM, Mehdi AMINI <joker.eph at gmail.com> wrote: >> >> >> >>

SGTM. I would do as 2 separate patches, the local per-module case, then the case that requires linker/global analysis. Thanks, Teresa On Fri, Feb 9, 2018 at 9:20 AM, Steven Wu via llvm-dev < llvm-dev at lists.llvm.org> wrote: > Are we agreeing on this is what we should do? If so, I will prepare a > patch. > > Steven > > > On Feb 8, 2018, at 11:07 AM, Mehdi AMINI

> On Feb 7, 2018, at 12:36 PM, Mehdi AMINI <joker.eph at gmail.com> wrote: > > > But it is interesting in general because according to the definition for local_unnamed_addr, the symbol has to be linkonce_odr to be auto hide as well. ThinLTO promotion can break that as well. > > What do you mean that the promotion can break that? > > The original description I found

> On Feb 7, 2018, at 4:03 PM, Mehdi AMINI <joker.eph at gmail.com> wrote: > > > > 2018-02-07 12:45 GMT-08:00 Steven Wu <stevenwu at apple.com <mailto:stevenwu at apple.com>>: > > >> On Feb 7, 2018, at 12:36 PM, Mehdi AMINI <joker.eph at gmail.com <mailto:joker.eph at gmail.com>> wrote: >> >> > But it is interesting in

Are we agreeing on this is what we should do? If so, I will prepare a patch. Steven > On Feb 8, 2018, at 11:07 AM, Mehdi AMINI <joker.eph at gmail.com> wrote: > > > > 2018-02-08 10:44 GMT-08:00 Steven Wu <stevenwu at apple.com <mailto:stevenwu at apple.com>>: > > >> On Feb 8, 2018, at 10:28 AM, Mehdi AMINI <joker.eph at gmail.com

On Thu, Feb 8, 2018 at 10:44 AM, Steven Wu via llvm-dev < llvm-dev at lists.llvm.org> wrote: > > > On Feb 8, 2018, at 10:28 AM, Mehdi AMINI <joker.eph at gmail.com> wrote: > > > > 2018-02-08 9:33 GMT-08:00 Steven Wu <stevenwu at apple.com>: > >> >> >> On Feb 7, 2018, at 4:03 PM, Mehdi AMINI <joker.eph at gmail.com> wrote: >>

The one that doesn’t require global analysis is commit in r324757. The global analysis will require adding a field into GlobalSummary. Let me do some research. The implementation should not be hard but I don’t want to break bitcode compatibility if that applies to the summary. Steven > On Feb 9, 2018, at 9:33 AM, Teresa Johnson <tejohnson at google.com> wrote: > > SGTM. I would

I wonder whether we could change the frontend to always give hidden visibility to globals which are linkonce_odr unnamed_addr. That way, the mechanism would just work in ELF as well and would not require LTO. Peter On Fri, Feb 9, 2018 at 10:46 AM, Steven Wu via llvm-dev < llvm-dev at lists.llvm.org> wrote: > The one that doesn’t require global analysis is commit in r324757. > >