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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
