nickdesaulniers added a comment.

In D104475#2825795 <https://reviews.llvm.org/D104475#2825795>, @MaskRay wrote:

> 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 complication/fallout initially, I'd prefer that we keep the initial 
> semantics narrow.

The guarantee is more important than the optimization.

> 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 )

IIUC, isn't @jdoerfert arguing that `optnone` and `noinline` being composable 
is _bad_?

>> 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.

Hyperbole. Only a few such function attributes would need such restrictions.

> 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.
>
> For example, if a kernel function can sometimes be inlinable, sometimes not: 
> add a alias and add `__attribute__((noinline))` to that alias.

Interesting idea, but `noinline` is straight up broken in LLVM: 
https://godbolt.org/z/MoE1a7c3v. The Linux kernel relies on `noinline` meaning 
don't perform inline substitution.  That violates the principal of least 
surprise.  When a developer uses `noinline`, `no_stack_protector`, or 
`no_profile`, they mean DO NOT inline, insert a stack protector, or insert 
profiling/coverage instrumentation EVER.

Also debugging to find out that inlining was the cause of such violated 
expectations sucks.


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