Hi, how do I force optimizations on an IR even when optnone is present on the functions? I know there is an option to force remove attributes, but for that I need to know the function names - is there a way to know the list of function names? I am working on a tool which is not in C/C++/python, so I do not have access to LLVM API, just the clang and opt binaries, and the input to the tool is LLVM IR.
Thanks!
I don’t think there is any such option and such an option probably also wouldn’t make sense.
If you want to get some functions optimized even though they are marked as optnone you would have to remove the attribute from the functions before handing off the IR to opt
If you control how the input IR is generated from the original source, you can always keep it from having optnone added in the first place.
I don’t control the sources. I have the LLVM IR file and running an optimisation pass using my tool will be completely intentional on the users end - so I know that if the user is trying to run the optimisation pass with my tool, they definitely did not want the optnone - and if they are a beginner they probably don’t even know about optnone. In any case, if no such option exists I guess I’ll just emit a warning or error.
You can use opt -passes=forceattrs -force-remove-attribute=optnone to remove the attribute from all functions.
This is perfect! Thank you 
Could we add a check in opt that if every function definition has the optnone attributes, we emit a warning and suggest to run opt -passes=forceattrs -force-remove-attribute=optnone first?
Just FYI, there are some passes (IPO category) that completely ignore optnone attribute.
If that’s true, those are bugs that should be fixed.
I think the point is that for IPO passes, interactions between optnone and non-optnone functions are not really well defined. The most egregious case was addressed by requiring optnone to be paired with noinline but for non-inlining cases, we do not have a good story.
Even for IPO, passes must assume that an optnone function is a blackbox and cannot inspect its contents. That seems pretty unambiguous.
I wish that was unambiguous. But when it last came up that was debated & I lost momentum to deal with the fallout & try to get this addressed…
I was going to address this by adding the noipa attribute (to clang and LLVM IR) and then having optnone also require to be paired with noipa (at that point could probably remove the pairing with noinline - I /think/ noipa would accurately imply noinline/be strictly more general/powerful/restrictive).
I specifically lost steam in D101011 when it was suggested that rather than adding noipa to the definition of “is interposable” as GCC did, we’d need to add a new function that abstracts over both and visit every call to “is interposable” and replace it with this new thing and add more test coverage to test that each particular case generalized over interposable and noipa. That really didn’t seem worthwhile to me and I moved on to other things 
I agree with the view that it’s a bug for IPO passes to operate on optnone functions. Do we have an issue for that?
The LangRef does say that IPO passes will operate on optnone functions, strangely:
This function attribute indicates that most optimization passes will skip this function, with the exception of interprocedural optimization passes.
While I agree with this - it wasn’t the prevailing opinion last time I brought this up (at least for the IR attribute - though I think at least we were all in agreement that it should be the behavior for the clang source attribute): Revisiting/refining the definition of optnone with interprocedural transformations - #38 by jdoerfert
Hmm… yep, that was apparently documented in the original implementation/version (I thought that might’ve been added due to/after my clarifying discussion thread linked above)… which I guess adds some weight to other people’s understanding of the attribute.
Though the “optnone must go with noinline” and various patches that did inhibit interprocedural optimizations in the presence of optnone (I think Chandler at least committed one such patch) certainly points to some confusion/mixed understanding.
I do think adding noipa, removing noinline requirement for optnone (maybe renaming optnone to nointraopt or something - then maybe noipa should be nointeropt to match) and having clang source optnone map to nointeropt+nointraopt would be great. And if folks want to help me dig the ⚙ D101011 [Attr] Add "noipa" function attribute out (either help me justify that changing “isInterposable” should test this property to and we shouldn’t retest/touch every caller - or some help/discussion about how we should go about doing all that efficently and without risking backsliding, etc… - that’d be great)
Add noipa, or nointerop, yes please.
Remove noinline has to with optnone, yes please.
Rename optnone, I would not, but I don’t have strong feelings and I see the rational.
Map source optnone to noipa + optnone (or the alternative names), yes please.
After a decade, my memory has faded a bit, but there were certainly objections to making optnone apply to IPO at the time, although I don’t remember the specific scenarios arguing for that exception. (Optimizations in general and IPO in particular are not my strong suits.) The LangRef language goes back to the original optnone commit (377496bb) so it wasn’t something tacked on later. That commit doesn’t point to a Phab review, unfortunately, but I think previous replies have pointed to some of the debates.
+1 to reworking the attributes as described by @dblaikie and @jdoerfert.
@jdoerfert are you still of the opinion expressed in ⚙ D101011 [Attr] Add "noipa" function attribute that we should revisit all mayBeDerefined/change/test them using a new API call, or would you be OK with changing the definition of mayBeDerefined to include noipa as I’ve done in the review?
Because I really don’t like the idea of revisiting all those calls, and also having a separate API that would risk getting out of sync. It seems to me that mayBeDerefined is an exact match for noipa - that there aren’t places where we’d want one and not the other. & it seems like it’d make the codebase more brittle (risk divergence - some places checking mayBeDerefined others testing mayBeDerefinedOrNoIPA) and increase the work (& unfortunate intermediate states where not all mayBeDerefined places have been addressed - or a large patch that addresses them all in one go)
Here’s a rough summary, I think, of all the places that we’d have to touch (& it seems smaller than I’d expect, honestly, so I wonder if I’ve missed some fan-out somewhere):
GlobalValue::mayBeDerefined
- GlobalValue::isExactDefinition
- GlobalValue::hasExactDefinition
- AdressSanitizer.cpp
- Debugify.cpp
- ObjCARCAPElim.cpp
- Attributor.cpp * 2
- FunctionAttrs.cpp * 10
- DeadArgumentElimination.cpp
- ValueLatticeUtils.cpp
- Attributor.h (isFunctionIPOAmendable)
- Attributor.cpp * 2
- AttributorAttributes.cpp * 5
- OpenMPOpt.cpp
I wouldn’t be averse to renaming the function (any/all in the mayBeDerefined-isExactDefinition-hasExactDefinition chain) if it’s helpful - or if there’s an argument to be made for moving it somewhere up/down the stack (though mayBeDerefined seemed closest to the root - most likely to ensure it gets checked in all the places that matter, whereas moving it up to hasExactDefinition or the like might result in it being missed in some cases) - but I can’t think of a case where we want to test optnone independently of interposable & so I’m averse to keeping both functions around, or to revisiting existing callers except to update the name of the function. (& I don’t think it needs retesting for every call site)
Somewhat prior art: ⚙ D97735 [Globals] Treat nobuiltin fns as maybe-derefined. (when nobuiltin support was added to mayBeDerefined we didn’t revisit/retest every use to ensure it was correct with this new definition - admittedly the argument was made that this was actually within the definition of refinement/derefinement, but perhaps a similar argument can be made for optnone)
This is what I don’t understand. noipa means, do not run interprocedural passes on this function (for whatever reason). Maybe derefined means, the function might be replaced with a different optimized function at link time (but we can still look at attributes for example).
Those two things are not the same (or we could implement the attribute through modified linkage), no matter if we currently have users that want one but not the other.
Arguments along those lines are brittle, IMHO, since they basically allows us to conflate meaning based on current use cases. (This happened a lot with attributes and it caused so many problems down the line as we untangled 3 different things all merged into one spelling “because nothing else was needed at the time”). Given that noipa is not a thing, it is not surprising we have no use cases (other than the ones introduced with the patch and that happen to correlate with maybe derefined).
That all said, I’m happy to add the noipa check in that function if we rename the function (mayBeDerefinedOrNoIPA), thus eliminating the old one. If someone wants to add the old function back, we might have found the use case to treat them explicitly. If no such use case is to be found, everyone will use the single existing (renamed) function.
WDYT?
I think I’d be OK with looking at existing attributes on a noipa function - if that’s a natural composition. “Don’t do any /other/ interprocedural analysis, but I want to tell you these things about the function”. That’d still block deducing/adding new attributes because noipa would stop the analysis required to do so.
So I think in that sense these concepts would still be correlated at least as far as I can see.
Wouldn’t that then have knock-on effects, though, that we don’t want? (we don’t want to actually change the linkage of the function)
Yeah, I can see how that can go both ways. It can increase technical debt later if/when we find the concepts to be distinct. But equally I worry (certainly speaking from my own experience context working on debug info and the history of transformations dropping debug info because it’s not considered as much when writing them) that if noipa were a wholely separate concept, folks might be more likely to catch the (maybe more common) interposable/derefineable kind of concept but not remember to check noipa - and if they’re sufficiently correlated, it’d be helpful to be able to bring noipa “along for the ride” to avoid it getting left behind.
Referring to optnone/noipa, or something else? Honestly I don’t think it would’ve been a bad thing if optnone had originally encapsulated both semantics - it’s what the users wanted at the Clang level, it’s what several LLVM developers assumed had happened anyway - and it would’ve served us well with fewer bugs/misunderstandings/discussions for the better part of a decade until someone might’ve decided to break it apart into separate inter and intraprocedural pieces, if that ended up being worthwhile (I’m not sure it would’ve been today - might’ve taken quite a while longer before that was sufficiently interesting to motivate the work)
Yep, sounds good to me - thanks for talking it through. I know there’s a lot of issues to weigh/contrast here, for sure.
I’ll update the patch with a renaming. (as always, open to better names - that was just the obvious one for the purposes of the conversation, at least)