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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to