[llvm-dev] Revisiting/refining the definition of optnone with interprocedural transformations

[David Blaikie via llvm-dev]

There seems to be a bunch of confusion and probably some
conflation/ambiguity about whether we're talking about IR constructs
on the C attributes.

Johannes - I assume your claim is restricted mostly to the IR? That
having optnone not require or imply noinline improves orthogonality of
features and that there are reasonable use cases where one might want
optnone while allowing inlining (of the optnone function) or optnone
while disallowing inlining (of the optnone function)

Paul - I think you're mostly thinking about/interested in the specific
source level/end user use case that motivated the initial
implementation of optnone. Where, I tend to agree - inlining an
optnone function is not advantageous to the user. Though it's possible
Johannes 's argument could be generalized from IR to C and still
apply: orthogonal features are more powerful and the user can always
compose them together to get what they want. (good chance they're
using attributes behind macros for ease of use anyway - they're a bit
verbose to write by hand all the time)

There's also the -O0 use of optnone these days (clang puts optnone on
all functions when compiling with -O0 - the intent being to treat such
functions as though they were compiled in a separate object file
without optimizations (that's me projecting what I /think/ the mental
model should be) - which, similarly, I think will probably want to
keep the current behavior (no ipa/inlining and no optimization -
however that's phrased).

Essentially the high level use cases of optnone all look like "imagine
if I compiled this in a separate object file without LTO" - apparently
even noipa+optnone isn't enough for that, maybe (I need to test that
more, based on a comment from Johannes earlier that inexact
definitions don't stop the inliner... )? Sounds like maybe it's more
like what I'm thinking of is "what if this function had weak linkage"
(ie: could be replaced by a totally different one)?

On Thu, Apr 22, 2021 at 9:23 AM Johannes Doerfert
<johannesdoerfert at gmail.com> wrote:
>
>
> On 4/22/21 11:05 AM, paul.robinson at sony.com wrote:
> >> On 4/22/21 9:43 AM, paul.robinson at sony.com wrote:
> >>>> That said, I believe it is a mistake that `optnone` requires
> >>>> `noinline`. There is no reason for it to do so on the IR level.
> >>>> If you argue C-level `optnone` should imply `noinline`, that is
> >>>> a something worth discussing, though on the IR level we can
> >>>> decouple them. Use case, for example, the not-optimized version
> >>>> is called from functions that are `optnone` themselves while
> >>>> other call sites are inlined and your function is optimized. So
> >>>> you can use the two attributes to do context sensitive `optnone`.
> >>> The original intent for `optnone` was to imitate the -O0 pipeline
> >>> to the extent that was feasible.  The -O0 pipeline (as constructed
> >>> by Clang) runs just the always-inliner, not the regular inliner;
> >>> so, functions marked `optnone` should not be inlined.  The way
> >>> to achieve that effect most simply is to have `optnone` require
> >>> `noinline` and that's what we did.
> >>>
> >>> If we have `optnone` stop requiring `noinline` and teach the
> >>> inliner to inline an `optnone` callee only into an `optnone` caller,
> >>> then we are violating the intent that `optnone` imitate -O0, because
> >>> that inlining would not have happened at -O0.
> >> I think I initially read this wrong, hence the part below.
> >> After reading it again, I have one question: Why would the
> >> inliner inline something that is not `always_inline` into
> >> an `optnone` caller? That would violate the idea of `optnone`,
> >> IMHO, regardless if the callee is `optnone` or not. That is
> >> why I don't believe `noinline` on the callee is necessary
> >> for your use case.
> > The inliner should be ignoring `optnone` callers, so it would
> > never inline *anything* into an `optnone` caller.  (Other than
> > an `alwaysinline` function.)
>
> My point is, it already does ignore `optnone` callers
> and inlines only `alwaysinline` calls into them:
>
> https://clang.godbolt.org/z/fznbjTEd5
>
>
> >
> > I had read this:
> >
> >>>> I believe it is a mistake that `optnone` requires `noinline`.
> > and the case that came to mind is inlining an `optnone` callee
> > into a not-`optnone` caller.  The inlined copy would then be
> > treated to further optimization, which violates the idea of
> > `optnone`.
>
> But that is a composition issue. If you do not want to
> inline a `optnone` callee into an non-`optnone` caller,
> then add `noinline` to the callee. If you don't mind if
> it is inlined into non-`optnone` callers and optimized
> in there, then don't. My last email contained an example
> to show the different cases, you can mix and match IR
> attributes to get what you want. Requiring them to be
> tied is not improving anything but just restricting the
> options.
>
>
> > Now, the inliner already knows to avoid `noinline` callees, so
> > attaching `noinline` to `optnone` functions was (at the time)
> > considered an optimal way to avoid the problematic case.  We
> > could instead teach the inliner to skip `optnone` callees, and
> > that would allow us to eliminate the requirement that `optnone`
> > functions must also be `noinline`.  I am unclear why redefining
> > `optnone` to _imply_ `noinline` (rather than _require_ `noinline`)
> > is better, but then I don't work much with attributes.
>
> The inliner will not inline calls into an `optnone` caller
> if it is not necessary. As said before, that would violate
> the `optnone` idea for the caller, no matter what the callee
> looks like. So requiring  `noinline` on the callee seems
> to me like a workaround or an oversight.
>
> It is better to not require them together because you can
> actually describe more distinct scenarios. Please take
> another look at my example in the last email, it shows
> what is possible if you split them. Furthermore, `optnone`
> does by design imply `noinline` for the call sites in the
> caller, or at least nobody argued that it shouldn't. Thus,
> requiring `noinline` on the callee is simply unnecessary
> as it does not add any value.
>
>
> >
> > The notion of allowing an `optnone` caller to inline an `optnone`
> > callee sounds like it would also violate the intent of `optnone`
> > in that it should imitate -O0, where inlining is confined to
> > `alwaysinline` callees, and `optnone` is defined to conflict with
> > `alwaysinline` (because if you always inline something, you are
> > allowing it to have subsequent optimizations same as the caller,
> > which conflicts with `optnone`).
>
> Nobody said `optnone` callers should inline calls that are
> not always_inline, at least so far I have not seen that
> argument be made anywhere. I'll just skip this paragraph.
>
>
> >
> > So, if you want to undo the _requirement_ that `optnone` must
> > have `noinline`, but then redefine `optnone` such that it can't
> > be inlined anywhere, you've done something that seems to have no
> > practical effect.  Maybe that helps Attributor in some way, but
> > I don't see any other reason to be making this change.
>
> I do not want to say `optnone` cannot be inlined. `noinline`
> says it cannot be inlined. If you want it to not be inlined,
> use `noinline`, if you want it to not be optimized in it's
> own function, use `optnone`, if you want both, use both.
>
> The practical effect was literally showcased in my last email,
> please go back and look at the example.
>
> I don't know why the Attributor has to do with this, I'm happy
> to hear your thoughts on that though :)
>
> ~ Johannes
>
>
> > --paulr
> >
>
>