llvm dev - Apr 2021 - Revisiting/refining the definition of optnone with interprocedural transformations

On 4/22/21 12:37 PM, paul.robinson at sony.com wrote:
>> 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)
> Even at the IR level, I'd argue that inlining a function marked optnone
> is violating the contract that the function should not be optimized;
> because once it's inlined somewhere else, there's no control over
the
> optimization applied to the inlined instance.
>
> Not to say we couldn't redefine the IR optnone that way, but it really
> feels wrong to have 'optnone' actually mean 'optsometimes'.
It does not. Take `noinline` as an example. A `noinline` function
is not inlined, so far so good. Now a caller of a `noinline`
function might be inlined all over the place anyway.
What I try to say is that function attributes apply to the function,
not to the rest of the world. If you want to say: do not optimize this
code ever, not here nor anywhere else, use `optnone` + `noinline`. If
you want to the function symbol to contain unoptimized code so
you can debug it, use `optnone`.

Let's make my context sensitive debugging example more concrete:

static void f1(int x) { ... }
static void f2(int x) { ...; f1(x); ... }
static void f3(int x) { ...; f2(x); ... }
static void f4(int x) { ...; f3(x); ... }

static void broken() { f4(B); }
static void working() {
   for (int i = 0; i < 1<<20; ++i)
     f4(A);
}

void entry() {
   working();
   broken();
}

So, let's assume we crash somewhere in f1 when we reach it from
broken but not from working. To debug we want to avoid optimizing
f1-4 and broken. To do that we can add `optnone` to all 5 functions
and `noinline` to broken. The effect will be that we have untouched
code in the call chain we want to debug while we potentially/probably
have reasonably fast code in the context of working which allows us
to actually run this fast.

Right now, you can get that effect if you use `__attribute__((flatten))`
on working, however it reverses the problem. You are required to
"mark"
all context that should be fast, not the ones you want to debug. Both
can be useful (IMHO).

>
>> 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)
> I'm obviously finding it hard to imagine a real use-case for that...
> I mean, sure you can lay out cases and say in a rather theoretical
> way, here's this interesting thing that happens when you do this.
> Interesting things are interesting, but are they practical/useful?
> Any non-speculative, real-world applications?  The YAGNI principle
> applies here.
What about the above? I can totally imagine something like this.

> (I believe the original inspiration was an MSVC feature, actually.)
>
> As long as the existing Clang __attribute((optnone)) semantics don't
> change (i.e., continued to imply noinline) it won't affect my users;
> but I would *really* not want to change something like that on them,
> without a bonafide use-case that could be readily explained.
>
> Here's a real-world case that might help explain my resistance.
> Sony has a downstream feature that allows suppressing debug-info for
> inlined functions; the argument is that these are generally small,
> easily verifiable, and debugging sessions that keep popping down into
> them are annoying and distracting from looking at the real problem.
>
> Our initial implementation depending on whether the function was
> actually inlined.  For one thing, it was easy to identify inlined
> scopes, and just not emit them.  However, this was a terrible user
> experience, because whether step-in did or didn't happen was dependent
> on how the optimizer happened to feel that day.  Programmers had no
> control over their debugging experience.
>
> We changed this so that programmers could tell, by looking at their
> source code, whether debug info would be suppressed.  In effect it's
> a command-line option that implicitly adds 'nodebug' to a given set
> of cases (methods defined in-class, 'inline' keyword).
>
> So, anything that smacks of "you get different things depending on
> whether the compiler decided to inline your function" just makes me
> twitch.
>
> And that's what the "optnone doesn't mean noinline"
proposal does.
Let's take a step back for a second and assume we would have
always said `optnone` + `noinline` gives you exactly what you
get right now with `optnone`. I think we can explain that to
people, we can say, `optnone` will prevent optimization "inside
this symbol" and `noinline` will prevent the code to be copied
into another symbol. Every use case you have could be served by
adding these two attributes instead of the one you do now. Everyone
would be as happy as they are, all the benefits would be exactly
the same, no behavior change if you use the two together. That said,
it would open up the door for context sensitive debugging. Now you
can argue nobody will ever want to debug only a certain path through
their program, but I find that position requires a justification more
than the opposite which assumes people will find a way to benefit from
it.

>
>> 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).
> The -O0 case was so that you can mix -O0 with LTO and have it stick.
> Your as-if seems like a reasonable model for it.
I totally think -O0 can imply all three attributes, optnone, noipa, 
noinline.
That is not an issue as far as I'm concerned.

~ Johannes

>
> Thanks,
> --paulr
>

> > Not to say we couldn't redefine the IR optnone that way, but it
really
> > feels wrong to have 'optnone' actually mean
'optsometimes'.
> 
> It does not.
A point of phrasing:  Please do not tell me how I feel.
I say it feels wrong, and your denial does not help the conversation.

The word "none" means "none."  It does not mean
"sometimes."  Can we
agree on that much?

Redefining a term "xyz-none" to mean "xyz-sometimes" feels
wrong.
If you want an attribute that means "optsometimes" then it should
be a new attribute, with a name that reflects its actual semantics.
I am not opposed to that, but my understanding is that we have been
arguing about the definition of the existing attribute.
> Take `noinline` as an example. A `noinline` function
> is not inlined, so far so good.
And if the compiler decides it is useful to make copies/clones of
the function, those aren't inlined either.  The copies retain their
original attributes and semantics.  (Perhaps the compiler can make
copies to take advantage of argument propagation, or some such.  I
do not think this proposition is unreasonable.)
> Now a caller of a `noinline`
> function might be inlined all over the place anyway.
> What I try to say is that function attributes apply to the function,
> not to the rest of the world. If you want to say: do not optimize this
> code ever, not here nor anywhere else, use `optnone` + `noinline`. If
> you want to the function symbol to contain unoptimized code so
> you can debug it, use `optnone`.
You are making a severe distinction between the copy of the function
that happens not to be inlined, and the copies that have been inlined,
such that the inlined copies have lost their original properties.
But just as the copies of the `noinline` function retain `noinline` 
and are not inlined, I argue that the `optnone` function copies ought 
to retain `optnone` and not be optimized.

LLVM does not have a way to not-optimize part of a function, so we 
achieve the goal by not inlining `optnone` functions.

I dispute that the inlined copies of an `optnone` function should
lose that much of their original characteristics, and the rest of 
the disagreement follows from there.  But I have a suggestion to
offer below.

> Let's take a step back for a second and assume we would have
> always said `optnone` + `noinline` gives you exactly what you
> get right now with `optnone`. I think we can explain that to
> people, we can say, `optnone` will prevent optimization "inside
> this symbol" and `noinline` will prevent the code to be copied
> into another symbol. Every use case you have could be served by
> adding these two attributes instead of the one you do now. Everyone
> would be as happy as they are, all the benefits would be exactly
> the same, no behavior change if you use the two together. That said,
> it would open up the door for context sensitive debugging. Now you
> can argue nobody will ever want to debug only a certain path through
> their program, but I find that position requires a justification more
> than the opposite which assumes people will find a way to benefit from
> it.
I don't think "inside this symbol" is meaningful to most
programmers.
They see methods/functions, and the internal operation of compilers
(e.g., making copies of functions) are relatively mysterious.  I say
this as someone who has spent many decades helping programmers use my
compilers.

I don't dispute that you can invent a scenario where it could be useful;
I reserve the right to be unpersuaded that it would occur often enough
that people would think of and make use of the feature.
> I totally think -O0 can imply all three attributes, optnone, noipa,
> noinline.
I totally think -O0 can imply { opt-sometimes, noipa, noinline }; and
this combination can be an upgrade path away from the existing optnone.

Can we proceed on that basis?

Thanks,
--paulr