[PATCH] D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++

Tue May 14 06:42:16 PDT 2019

tejohnson added a comment.

In D61634#1501066 <https://reviews.llvm.org/D61634#1501066>, @gchatelet wrote:

> In D61634#1500453 <https://reviews.llvm.org/D61634#1500453>, @efriedma wrote:
>
> > My main blocker is that I want to make sure we're moving in the right direction: towards LLVM IR with clear semantics, towards straightforward rules for writing freestanding C code, and towards solutions which behave appropriately for all targets.  There's clearly a problem here that's worth solving, but I want to make sure we're adding small features that interact with existing features in an obvious way.  The patch as written is adding a new attribute that changes the semantics of C and LLVM IR in a subtle way that interacts with existing optimizations and lowering in ways that are complex and hard to understand.
>
>
> This makes a lot of sense, I'm totally on board to reduce entropy and confusion along the way.
>
> > I don't want to mix general restrictions on calling C library functions, with restrictions that apply specifically to memcpy: clang generally works on the assumption that a "memcpy" symbol is available in freestanding environments, and we don't really want to change that.
> > 
> > With -fno-builtin, specifically, we currently apply the restriction that optimizations will not introduce memcpy calls that would not exist with optimization disabled.  This is sort of artificial, and and maybe a bit confusing, but it seems to work well enough in practice.  gcc does something similar.
> > 
> > I don't really want to add an attribute that has a different meaning from -fno-builtin.  An attribute that has the same meaning is a lot less problematic: it reduces potential confusion, and solves a related problem that -fno-builtin currently doesn't really work with LTO, because we don't encode it into the IR.
>
> Adding @tejohnson to the conversation.
>
> IIUC you're offering to introduce something like `__attribute__((no-builtin-memcpy))` or `__attribute__((no-builtin("memcpy")))`.
>  As attributes they would compose nicely with (Thin)LTO.
>
> I believe we still want to turn `-fno-builtin` flags into attributes so there's no impedance mismatch between the flag and the attribute right?

I have a related patch that turns -fno-builtin* options into module flags, and uses them in the LTO backends. This addresses current issues with these flags not working well with LTO.
See https://reviews.llvm.org/D60162

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61634