search for: availableexternal

...pcc.me.uk/lib/Transforms/IPO/FunctionAttrs.cpp) to ThinLTO so it does something similar to what LTO is doing ( https://bugs.llvm.org/show_bug.cgi?id=33648). I've hit a problem with how the FunctionAttrs optimization expects linkage types. In ThinLTO since the linkage type is set to External or AvailableExternally it is unable to apply any of the functionattr optimizations that LTO can (since LTO is able to convert everything to Internal). This is a problem because every functionattr optimization expects an exact definition by calling hasExactDefinition (http://llvm-cs.pcc.me.uk/ include/llvm/IR/GlobalValu...

...cpp) to ThinLTO >> so it does something similar to what LTO is doing >> (https://bugs.llvm.org/show_bug.cgi?id=33648). I've hit a problem with how >> the FunctionAttrs optimization expects linkage types. >> >> In ThinLTO since the linkage type is set to External or AvailableExternally FunctionAttrs should still kick in for external linkage, can you post an IR sample of where that's not happening (since that's a bug IIUC)? As far as available_externally is concerned, preventing FunctionAttrs on them is by design and I'm not sure if there is anything in ThinLTO to...

On 8 Jun 2015, at 21:08, Eric Christopher <echristo at gmail.com> wrote: > > I'd rather not have it be an llvm option at all. Just construct a different set of passes... > > This would also solve the problem of needing multiple sets of options to be passed to the builder. It'd be a bit of a change (i.e. having clang do the pass setup), but I think it'd be worth it

On Thu, Jun 4, 2015 at 5:02 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote: > >> On 2015 Jun 4, at 16:51, Reid Kleckner <rnk at google.com> wrote: >> >> On Thu, Jun 4, 2015 at 3:58 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote: >> > Personally, I think the right approach is to add a bool to createGlobalDCEPass defaulting to

On Thu, Jun 4, 2015 at 11:27 AM, Duncan P. N. Exon Smith < dexonsmith at apple.com> wrote: > Since the compiler is always free to delete available_externally > functions, I think you could just add a pass to the -flto=thin pipeline > that deletes all of them (referenced or not) -- it's just a single loop > through all the functions deleting the bodies of those with the right

...nal and > Private linkages), where it changes to External as described in Section > 4.3.1. In all other cases the linkage in the original module stays the > same, and therefore is not noted in the table below. > > > > Note that there are no InternalLinkage, PrivateLinkage, > AvailableExternallyLinkage, LinkOnce*Linkage, Weak*Linkage, > AppendingLinkage and CommonLinkage declarations. There are no > ExternalWeakLinkage definitions. > > > > > +---------------------------------+------------------------------------------------+ > > |...

...the imported copy. 2.2 Linkage Effects There is no change to the linkage type required in the original module for any non-discardable values that may be imported to another module. During importing, ideally all non-discardable definitions have their linkage type changed in the imported copy to AvailableExternallyLinkage. This signals to the compiler that the definition can safely be eliminated after inlining (i.e. by the new EliminateAvailableExternally pass). In practice, we do not change the linkage for all imported non-discardable defs. The linkage effects for the non-discardable linkage types are desc...

...> > Some more prior discussion: https://reviews.llvm.org/D19995#423481 > > +1 So we're all on the same page, our current encodings are: External: 0 WeakAny: 16 Appending: 2 Internal: 3 LinkOnceAny: 18 ExternalWeak: 7 Common: 8 Private: 9 WeakODR: 17 LinkOnceODR: 19 AvailableExternally: 12 And the new encodings would be: External: 0 External + Interposible: 20 Weak + Interposible: 16 Appending: 2 [interposible N/A] Internal: 3 [never interposible] LinkOnce + Interposible: 18 ExternalWeak: 7 [always interposible] Common: 8 [interposible N/A] Private: 9 [never interpo...

...gt; > > There is no change to the linkage type required in the original module > for any non-discardable values that may be imported to another module. > > > > During importing, ideally all non-discardable definitions have their > linkage type changed in the imported copy to AvailableExternallyLinkage. > This signals to the compiler that the definition can safely be eliminated > after inlining (i.e. by the new EliminateAvailableExternally pass). In > practice, we do not change the linkage for all imported non-discardable > defs. The linkage effects for the non-discardable li...

...::op_iterator U = I->op_begin(), E = I->op_end(); U != E; ++U) >> - if (GlobalValue *GV = dyn_cast<GlobalValue>(*U)) >> - GlobalIsNeeded(GV); >> + if (GlobalValue *GV = dyn_cast<GlobalValue>(*U)) { >> + if (!GV->hasAvailableExternallyLinkage()) >> + GlobalIsNeeded(GV); >> + } >> else if (Constant *C = dyn_cast<Constant>(*U)) >> MarkUsedGlobalsAsNeeded(C); >> } > > > IIUC, this changes the logic from "if it's referenced, keep it&...

...update the comment. > > + MPM.add(createElimAvailExternPass()); > + } > > + // Drop initializers of available externally global variables. > + for (Module::global_iterator I = M.global_begin(), E = M.global_end(); > + I != E; ++I) { > + if (!I->hasAvailableExternallyLinkage()) > + continue; > + if (I->hasInitializer()) { > + Constant *Init = I->getInitializer(); > + I->setInitializer(nullptr); > + if (isSafeToDestroyConstant(Init)) > + Init->destroyConstant(); > + } > + I->removeDeadCo...

On Thu, Jun 4, 2015 at 5:33 PM, Reid Kleckner <rnk at google.com> wrote: > On Thu, Jun 4, 2015 at 5:17 PM, Teresa Johnson <tejohnson at google.com> wrote: >> >> Agreed. Although I assume you mean invoke the new pass under a >> ThinLTO-only option so that avail extern are not dropped in the >> compile pass before the LTO link? > > > No, this pass

...t looks like one of >>>>>>>>>>> the >>>>>>>>>>> places I had changed (where we mark defined globals in >>>>>>>>>>> runOnModule) >>>>>>>>>>> already has a guard for !hasAvailableExternallyLinkage and >>>>>>>>>>> !isDiscardableIfUnused, so my additional guard against marking >>>>>>>>>>> imported functions is unnecessary. But the other place I had to >>>>>>>>>>> change >>>>&...

...nson at google.com> >>> > wrote: >>> >> >>> >> Looking back through my GlobalDCE changes, it looks like one of the >>> >> places I had changed (where we mark defined globals in runOnModule) >>> >> already has a guard for !hasAvailableExternallyLinkage and >>> >> !isDiscardableIfUnused, so my additional guard against marking >>> >> imported functions is unnecessary. But the other place I had to change >>> >> was in GlobalIsNeeded where it walks through the function and >>> >> re...

On Thu, Jun 4, 2015 at 3:58 PM, Duncan P. N. Exon Smith < dexonsmith at apple.com> wrote: > > > Personally, I think the right approach is to add a bool to > createGlobalDCEPass defaulting to true named something like > IsAfterInlining. In most standard pass pipelines, GlobalDCE runs after > inlining for obvious reasons, so the default makes sense. The special case > is

...ide to import that definition, the thin link analysis could decide that it is better to leave all copies local. The index would indicate local linkage, and the linkage type in the IR would then be changed to local when it is imported (and when compiling the exporting module) D. LinkOnce -> Weak/AvailableExternally: This is a compile time optimization to avoid unnecessarily keeping multiple copies of a LinkOnce value. Linkage is marked in index, and again adjusted in the backends since it will be different than the initial linkage after parsing. Note that pcc has made a proposal to do some of the ThinLTO p...

...GlobalDCE changes, it looks like one of >> >> >>> >> the >> >> >>> >> places I had changed (where we mark defined globals in >> >> >>> >> runOnModule) >> >> >>> >> already has a guard for !hasAvailableExternallyLinkage and >> >> >>> >> !isDiscardableIfUnused, so my additional guard against marking >> >> >>> >> imported functions is unnecessary. But the other place I had to >> >> >>> >> change >> >> >>> &...

...() { return new ElimAvailExtern(); } + +bool ElimAvailExtern::runOnModule(Module &M) { + bool Changed = false; + + // Drop initializers of available externally global variables. + for (Module::global_iterator I = M.global_begin(), E = M.global_end(); + I != E; ++I) { + if (!I->hasAvailableExternallyLinkage()) + continue; + if (I->hasInitializer()) { + Constant *Init = I->getInitializer(); + I->setInitializer(nullptr); + if (isSafeToDestroyConstant(Init)) + Init->destroyConstant(); + } + I->removeDeadConstantUsers(); + NumVariables++; + }...

...> wrote: >> >>> >> >> >>> >> Looking back through my GlobalDCE changes, it looks like one of the >> >>> >> places I had changed (where we mark defined globals in runOnModule) >> >>> >> already has a guard for !hasAvailableExternallyLinkage and >> >>> >> !isDiscardableIfUnused, so my additional guard against marking >> >>> >> imported functions is unnecessary. But the other place I had to >> >>> >> change >> >>> >> was in GlobalIsNeeded where...

I think that all makes sense. You're just adding the missing non-ODR conterpart of 'external' linkage. I could imagine having "external / external_odr" linkage for example. That said, do you think we should take the opportunity to split out a bit for interposability so that we can kill off the *_odr linkage variants? Today's non-ODR weak functions would look more like