[PATCH] D101011: [Attr] Add "noipa" function attribute

Tue Mar 15 09:37:14 PDT 2022

dblaikie added a comment.

In D101011#2715555 <https://reviews.llvm.org/D101011#2715555>, @MaskRay wrote:

> This attribute will be useful:) Thanks for working on it.
>
> I agree with @jdoerfert (https://lists.llvm.org/pipermail/llvm-dev/2021-April/150062.html) that the proposed attribute, noinline, and optnone should be orthogonal: no one implies the other(s).

At the IR level, I think that's fine/fair.

> If my reading from the long mailing list thread is correct, it is still undecided how the clang attribute should behave.

I think Clang optnone -> IR optnone + noipa.

> (Personally I'd hope that the GCC attribute `noipa` ("This attribute is supported mainly for the purpose of testing the compiler.") were different from the GCC attribute `noinline`.)

Sure - I forget the context, but I don't think there's any reason/need to have noipa and noinline alias each other, they are distinct features. (though, unfortunately, sometimes IPA produces the equivalent of inlining ("hey, this function always returns 5, we can just use 5 at the call site - oh, also the function has no side effects, so we can remove the now-unused call... and we've essentially inlined, even though we didn't use the inliner") - not sure if there are some IPA features we could intentionally break that'd stop LLVM doing "the equivalent of inlining, without using the inliner")

noipa will effectively imply the equivalent of noinline - because it'd be impossible to inline in the absence of a definition. But the inverse isn't true. I don't think we necessarily have to lower a clang-level noipa to an IR noipa+noinline, I think just using noipa would be sufficient to get the non-inlining semantics (again: how could you inline if you can't see the definition of a function).

> If the clang attribute would end up combining two attributes, we should better make the IR/clang attribute names different.

Perhaps. Though probably the thing to rename would be optnone, rather than noipa. I think noipa is the right name. I think optnone as a name should/does include noipa. (I think this whole rabbit hole we got down to here is unfortunate and previous understanding of optnone did include noipa-like behavior (I think I've referenced previous patches that were consistent with that understanding & the original author (& myself as one of the reviewers of the feature) have stated that that was the intent of optnone, to also be noipa-like) - but I can appreciate that having distinct attributes can provide greater flexibility)

> `nointeropt` may be a better name anyway because analysis in ipa does not necessarily mean optimization? :)

Eh, I don't think I'd want to go down that path - I think it's important to model this as noipa, as "imagine this function definition were not available at all" (as though it were defined in another translation unit entirely) - no analysis, no optimization, nothing - you can't see the definition at all. It's an easy to explain, robust concept.

> If our clang attribute is likely to have different semantics (I'd favor orthogonal semantics), `noipa` would be inappropriate as it will conflict with the GCC semantics.

I don't think there's any need for our noipa to differ from GCC's semantics - their documentation that says noipa implies noinline can be seen as a description of behavior, not a constraint on how that behavior is achieved. noipa should logically disable inlining as a consequnce of disallowing interprodecural analysis/information.

In D101011#3381751 <https://reviews.llvm.org/D101011#3381751>, @xbolva00 wrote:

> Re-ping

Sorry I haven't picked this up again. I lost steam when folks suggested that to implement this we could/should revisit/rewrite/retest every call site that checks for interposability... that just doesn't seem productive to me/hard to justify the time to do all that work and doing it partially I think would be really harmful (leaving interposability unaligned/inconsistent with noipa in various subtle ways).

Happy to talk about how we could move this forward perhaps with more hands to do all that work, but I still just don't feel great about that as a direction, really.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101011/new/

https://reviews.llvm.org/D101011