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

Tue Mar 15 11:01:38 PDT 2022

dblaikie added a comment.

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

> if llvm attribute noipa also adds logical assumption about noinline, we should not then audit all check of noinline whether they should be extended for noipa check, or?

No, I don't believe we do - if interposability is already well tested, that was the point of leveraging that definition/functionality - noipa should be very similar/the same as interposability. But if we have to revisit every check for interposability and rewrite the check - then, yes, we'd end up adding more test coverage and as a consequence we would end up adding testing for cases where noipa subsumes noinline, probably. (maybe the interposability testing is somewhere else/not in noinline directly - in which case we wouldn't add inlining testing, we'd test that other piece that happens to feed into inlining)

In D101011#3383074 <https://reviews.llvm.org/D101011#3383074>, @rnk wrote:

> I agree, this is useful functionality, we should add it.
>
> I would be OK with an all-in-one non-orthogonal `noipa` attribute. The only use case I can come up with for orthogonal attributes is for constructing fine-grained compiler test cases, or trying to carefully convince the optimizer to do one transform or another. The original use case seems more important.
>
> I would also like to suggest another use case for the original attribute, which is that this feature supports hotpatching. If you block IPO across a particular function boundary, you can more reliably recompile and hotpatch in new code, without going to great lengths to break up modules into smaller object files and relinking that way.

Ah, indeed.

> In D101011#3383003 <https://reviews.llvm.org/D101011#3383003>, @xbolva00 wrote:
>
>>>> 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.
>>
>> GCC’s __attribute__(noipa) (frontend) -> noipa, noinline, noclone, no_icf (midend).
>
> That has some appeal, but what about future optimizations? This supports testability (you can block each transform individually), but it requires frontends to keep track of these unpacked attributes that block all interprocedural optimization. And there's a bitcode auto-upgrade problem: if the frontend intended to block all IPO, how to you upgrade that intention? If you ignore that, you can solve the frontend problem with a utility like `AttrBuilder::addNoIpaAttrs()` which adds all the relevant attributes (noinline, noicf, whatever).

I'm just not sure it makes sense to decompose these - like I don't know what noipa without noinline or no_icf would mean - if we're saying it's invalid IR /not/ to combine them, then I'm still confused by how we could/would implement noipa that wouldn't implicitly subsume toinline/no_icf functionality anyway. (if there was an implementation of noipa that didn't implicitly subsume the functionality of noinline/no_icf, then I'd consider that feature brittle/buggy in a way I don't think it could/should be - if noipa is tested at the same level as interposable, it should flow through inlining and icf naturally without the need for specific attributes to disable them)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101011