Revisiting/refining the definition of optnone with interprocedural transformations

Hi Johannes,

I've taken some time to try to understand your viewpoint,
and I will give some more of the history as I remember it;
hopefully that will help. And a suggestion at the end.

The point is, I never suggested to change the meaning of
`optnone` (in IR). I argue to change the requirement for
it to always go with `noinline`. `optnone` itself is not
changed, you get exactly the same behavior you got before,
and `noinline` is also not changed. They are simply not
required to go together.

Okay.

I can understand looking at the as-implemented handling of
'optnone' and taking that as the intended meaning. So from that
perspective, lacking the history, it does look to you like you are
not suggesting a change in its meaning.

Actually, removing the requirement to tie them together *is*
something that I would consider a semantic change, will require an
update to the LangRef and verifier, and so on. This would be more
obvious if we had originally done either of two things with the
same net semantic effect as we have now:
- make optnone *imply* noinline (like 'naked' does IIUC) instead
of *requiring* it.
- make the inliner check for optnone on the callee, instead of
simply tying the two attributes together.

I hope this helps explain why I see optnone+noinline as simply
two parts of one unified feature.

History:

The meaning of 'optnone' was always intended as, don't optimize,
in as many ways as we can manage. I'd rather have not had the
coupling with noinline, but it was the best way forward at the
time to achieve the effect we needed. I spent too many months of
my life getting this accepted at all...
Maybe my definition of optnone in the LangRef is inadequate; but
I am quite sure I understand the original intent.

Which includes this:

When it comes to the interaction of optnone and inlining, there
were of course four cases to consider: caller and callee, and each
might or might not have optnone.

1) caller - N, callee - N
2) caller - N, callee - Y
3) caller - Y, callee - N
4) caller - Y, callee - Y

1) normal case. Inlining and other opts are okay.
2) callee is optnone, so we didn't want it inlined and optimized.
3) caller is optnone, so no inlining happens.
4) caller is optnone, so no inlining happens.

Cases 3/4 are handled by checking for optnone in the inliner pass,
just like a hundred other passes do. This is boilerplate and was
added in bulk to all those passes. Having it all be boilerplate
was important in getting the reviews accepted.

Case 2 was handled by coupling optnone to noinline. As I said,
this was a practical thing done at the time, not because we ever
intended the semantics of optnone to mean anything else. It got
the overall feature accepted, which was the key thing for Sony.

So, what we implemented achieved the effect we wanted, even if
that implementation didn't mean the new attribute *by itself* had
exactly all the effects we wanted.

Yes, decoupling optnone (as implemented) from noinline would allow
case 2 to inline bar into foo, and optimize the result, if that
somehow seems desirable. But, that result is very much against
the original intent of optnone, and is why I have been giving you
such a hard time about this.

I will continue to insist that something called 'optnone' cannot
properly fail to apply to all instances, inlined or not. But if
we're willing to rename the attribute, that problem is solved.

I'm assuming there would be resistance to doing either of the two
things I mentioned at the top (make optnone *imply* noinline, or
modify the inliner to check for optnone on the callee) as that
doesn't seem to be the direction people are moving in.

So the suggestion is:
- reword the definition of optnone to be something that would be
better named "nolocalopt";
- ideally, actually rename the attribute (because I still say
that "none" does not mean "unless we've inlined it");
- and yes, if you must, decouple it from noinline and remove that
paragraph from the LangRef description.

Clang will still pass both IR attributes, so end users won't see
any feature regressions. David can add 'noipa' and we can make
Clang pass that as well.

Works for me.

~ Johannes

I see that the clang attribute 'optnone' patch rC205255 (in 2014) added
both the IR 'optnone' and 'noinline' attributes.

If the clang attribute 'optnone' (for debugging purposes) is to be renamed,
I humbly suggest we may consider implementing __attribute__((optimize("O0")))
(limited to "O0" only; other values are not accepted).

I mentioned that somewhere else, maybe not on the list though, but we are looking
into the option to select the optimization level per function. Basically, remove
the limit to O0 in `__attribute__((optimize("OX"))`. I'll start a new thread once
we are closer where we explain why exposing it to the user is only one use case.
Long story short, `optimize("O0")` would be nice and we could use it for debugging
as suggested :wink:

~ Johannes