MaskRay added a comment. In D104475#2825841 <https://reviews.llvm.org/D104475#2825841>, @nickdesaulniers wrote:
> 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_? No, the opposite. @jdoerfert is arguing that attributes should be composable. I agree and I think the `optnone` design has some orthogonality problems. >>> 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. It is not broken. That is constant propagation in early-cse. A function call can be optimized out if it has no side effect and its value can be determined. The behavior may look like inlining. That is the `nointeropt` IR attribute proposal intends to address. I guess for most(all?) non-trivial noinline usage in the Linux kernel, we can assume `noinline` is fine. 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
