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

[Johannes Doerfert via llvm-dev]

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
>