[PATCH] D74162: [Inliner] Inlining should honor nobuiltin attributes

[Joseph Huber via Phabricator via llvm-commits]

jhuber6 added a comment.

In D74162#4518301 <https://reviews.llvm.org/D74162#4518301>, @gchatelet wrote:

> Adding @sivachandra for the LLVM libc part.
>
> The gist of the issue is described in the RFC <https://lists.llvm.org/pipermail/llvm-dev/2019-April/131973.html>.
>
> A few considerations first:
>
> - LLVM libc is not written in assembly because we want compiler support for optimizations, sanitizers and fuzzers.
> - Compiler is able to recognize C/C++ constructs and turn them into library calls unless we use the dedicated `-fno-builtin-*` compiler flags.
> - `-ffreestanding` implies `-fno-builtins` (i.e., all builtins).
>
> Now let's consider the case of a `foo` function calling `memcpy` where we allow inlining of functions with different attributes.
> `foo` is compiled without any of `-ffreestanding`/`-fno-builtin-*`.
> During LTO, the compiler decides to inline all of the code of `memcpy` within `foo`, discarding `memcpy`'s `-fno-builtin-memcpy` flag (stored as a function attribute).
> Deep down `memcpy`'s implementation the compiler recognizes a copy loop and turns it into the `@llvm.memcpy` IR intrinsic.
> The backend decides that this intrinsic is best implemented by calling libc's `memcpy` and inserts a call.
> Now `memcpy` is implemented by a call to itself and will eventually stack overflow or infinite loop.
> This happened in production.
>
> Now for `memcpy` in particular we have introduced `__builtin_memcpy_inline` (doc <https://clang.llvm.org/docs/LanguageExtensions.html#guaranteed-inlined-copy>) which allows us to convey the semantics of "fixed size operations that always generate code". They are guaranteed to never "call to the libc" (godbolt <https://godbolt.org/z/4f8M9zqq7>). Unfortunately those intrinsics are not supported in GCC so this would be clang only. Also they don't yet cover `memcmp` and `bcmp`.
>
> Today only `memcpy` relies completely on `__builtin_memcpy_inline` (when compiled with clang) so technically this function could be compiled without `-ffreestanding` nor `-fno-builtin` and thus inlined during LTO.
>
> Now it is very desirable that the libc functions can be inlined through LTO, we may be able to reach this state when we have strong guarantees that the compiler will not generate libc calls from within the libc function themselves. We're there for `memcpy` which really is the most fundamental libc function. But we're not there for most of the libc.
>
> Note: for the reasons described above we had to implement the `no-builtin` flags as function attributes so they can stick to code even during LTO.

Thanks for the background. Was there a reason that we could not simply merge the `nobuiltin` attribute into the caller? By the time we get to this point it's likely that we've already run optimizations on the TUs individually so I'd guess that we wouldn't lose any potential optimizations more severe than not being able to inline the function in the first place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74162