[libc-commits] [PATCH] D72102: [libc] Add __attribute__((always_inline)) to x86_64 syscall functions.

[Siva Chandra via Phabricator via libc-commits]

sivachandra added a comment.

In D72102#1806840 <https://reviews.llvm.org/D72102#1806840>, @abrachet wrote:

> In D72102#1806753 <https://reviews.llvm.org/D72102#1806753>, @sivachandra wrote:
>
> > In D72102#1806672 <https://reviews.llvm.org/D72102#1806672>, @abrachet wrote:
> >
> > > Shouldn't these all be `static` as well, actually?
> >
> >
> > We really want these functions to be inlined. So, I am not sure how static helps in such scenarios. If they are not inlined, there are probably some advantages in ELF land, but we end up with multiple copies.
>
>
> Neither `inline` nor `__attribute__((always_inline))` guarantee the symbol will not be emitted. From a quick test it looks like adding `inline` with `always_inline` never emitted the symbol but neither of these on their own have have that behavior. https://godbolt.org/z/R5Q-Ft. This is consistent with basically every compiler on godbolt, so it might be safe to leave `static` off and keep both `inline` and `__attribute__((always_inline))` but I don't understand why these two together would mandate this so I wouldn't personally feel safe relying on that. I'm sure @MaskRay knows, though.


IIUC, if a direct call to function marked `always_inline` does not cause inlining, it is a diagnostic error. For indirect function calls though, the compiler will not inline and will also not report it. I think we have examples for both the scenarios in the libc tree: the mmap and munmap implementations make a direct call, while the x86_64 specific test takes the address of these syscall functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72102