[PATCH] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

Fangrui Song via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 17 15:26:01 PDT 2021


MaskRay added a comment.

In D104475#2825772 <https://reviews.llvm.org/D104475#2825772>, @nickdesaulniers wrote:

> In D104475#2825711 <https://reviews.llvm.org/D104475#2825711>, @MaskRay wrote:
>
>> So if we don't want to offer guarantee for IR/C/C++ attributes, we can document that users may need to additionally specify `__attribute__((noinline))` to suppress inlining.
>
> I don't generally like that approach:

If a guarantee would cause inferior optimization or we cannot think of all the fallout initially, I'd prefer that we keep the initial semantics narrow.
If we cannot make a promise, don't provide a promise.
No-suppressing inlining makes the attribute freely composable with other attributes (https://lists.llvm.org/pipermail/llvm-dev/2021-April/150062.html )

> 1. it's not easy for developers to validate their call call chains to ensure that a caller with a restrictive function attribute doesn't have unrestricted callers inlined into the restricted caller.
> 2. that behavior opposes the principle of least surprise.

Disagree. If the user wants a function not to be inlined, let them add `__attribute__((noinline))`.

> 3. We don't have a statement attribute for call sites to say "please don't inline this call" which would be fine grain. noinline is a course-grain hammer; what if we want to inline a callee into most callers but not this one caller that has such a restricted function attribute?



> See also D91816 <https://reviews.llvm.org/D91816> where I took the conservative approach for `no_stack_protector` and simply chose not to perform such inline substitution on caller and callee function attribute mismatch.  I find this behavior to be the least surprising, and the developer is provided with introspection as to why the compile did not perform such inlining via `-Rpass=inline` flag.

I think D91816 <https://reviews.llvm.org/D91816> was an unfortunate case. That would require the inliner to understand every possible attribute. That was the practical approach back then because it is not easy for the kernel to cope.
For new things we should strive for making the kernel do the right thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104475



More information about the cfe-commits mailing list