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

Tue Mar 15 10:29:03 PDT 2022

dblaikie added a comment.

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).
>
> https://github.com/gcc-mirror/gcc/commit/036ea39917b0ef6f07a7c3c3c06002c73fd238f5
>
> Any good reason why not do it same way as well? Avoid hidden logical assumption that noipa is basically noinline + something.

Because I don't think it should be necessary - noipa semantics should be strictly more powerful than noinline, so there shouldn't be a need to put noinline on the function.

If noinline ever makes a difference on a noipa function, I think that's a pretty serious bug.

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

>>> 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).
>
> :/ not productive approach to work on one perfect patch.

I don't think the approach of revisiting/retesting all these call sites is the right way to go about things. So I stopped working on this because I couldn't think of/find a way to approach this that addressed both my perspectives and those of reviewers here.

> This work could be split easily. 1) Basic llvm support, 2) clang attribute, 3) complete llvm support.

The intermediate state created by pieces of (3) I have two concerns with:

1. I don't think it's a productive use of time to retest and rewrite every check for interposability
2. It introduces (certainly initially) divergence between these two properties which I don't think should ever diverge

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101011