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

tejohnson added a comment. The last patch from my side just went in (D74162 : [Inliner] Inlining should honor nobuiltin attributes). So I think this is now complete. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

gchatelet abandoned this revision. gchatelet added a comment. This has been implemented in the following patches: - https://reviews.llvm.org/D67923 - https://reviews.llvm.org/D74162 - https://reviews.llvm.org/D73543 - https://reviews.llvm.org/D71710 Repository: rG LLVM Github Monorepo

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

gchatelet added a comment. Thx for the summary @tejohnson. In D61634#1808265 , @tejohnson wrote: > >> 3. Propagate/merge the `no-builtin` IR attribute for LTO by "updating > >> `AttributeFuncs::areInlineCompatible` and/or > >>

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

tejohnson added a comment. In D61634#1515176 , @tejohnson wrote: > In D61634#1512020 , @gchatelet wrote: > > > AFAIU here is a coarse plan of what needs to happen > I've listed below what I believe is the status:

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

jdoerfert resigned from this revision. jdoerfert added a comment. This has enough active reviewers as it is. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61634/new/ https://reviews.llvm.org/D61634

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

gchatelet added a comment. In D61634#1732884 , @tejohnson wrote: > In D61634#1682660 , @gchatelet wrote: > > > In D61634#1679331 , @tejohnson > > wrote: > > > > > In

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

tejohnson added a comment. In D61634#1682660 , @gchatelet wrote: > In D61634#1679331 , @tejohnson wrote: > > > In D61634#1635595 , @tejohnson > > wrote: > > > > > I had

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

gchatelet added a comment. In D61634#1679331 , @tejohnson wrote: > In D61634#1635595 , @tejohnson wrote: > > > I had some time to work on this finally late last week. I decided the most > > straightforward thing

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

tejohnson added a comment. In D61634#1635595 , @tejohnson wrote: > I had some time to work on this finally late last week. I decided the most > straightforward thing was to implement the necessary interface changes to the > TLI analysis to make it

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

tejohnson added a comment. Herald added a reviewer: jdoerfert. I had some time to work on this finally late last week. I decided the most straightforward thing was to implement the necessary interface changes to the TLI analysis to make it require a Function (without any changes yet to how that

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

gchatelet added a comment. In D61634#1586047 , @tejohnson wrote: > Checking in to see where we are on this issue. I haven't had any time to work > on 4 but hopefully can start on that soon. But it needs the first part done > to be effective. Thx for

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

tejohnson added a comment. In D61634#1515176 , @tejohnson wrote: > In D61634#1512020 , @gchatelet wrote: > > > AFAIU here is a coarse plan of what needs to happen > > > > 1. Add a `no-builtin` clang function

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

gchatelet updated this revision to Diff 203386. gchatelet added a comment. - Add documentation. - Fix permissive HasNoRuntimeAttribute - Mark interleave as disabled in the documentation. - Use no-builtin instead of no-runtime-for. - Adding an llvm.memcpy.inline intrinsic. - Adding

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

kristina added a comment. In D61634#1517228 , @xbolva00 wrote: > I have a question about qsort.. If we provide own implementation of qsort and > replace calls to libc's qsort to our qsort, we could fully inline cmp > function then. Ideas? `qsort`

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

xbolva00 added a comment. I have a question about qsort.. If we provide own implementation of qsort and replace calls to libc's qsort to our qsort, we could fully inline cmp function then. Ideas? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

tejohnson added a comment. In D61634#1512020 , @gchatelet wrote: > AFAIU here is a coarse plan of what needs to happen > > 1. Add a `no-builtin` clang function attribute that has the same semantic as > the `no-builtin` cmd line argument >

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

gchatelet updated this revision to Diff 200998. gchatelet added a comment. - Use no-builtin instead of no-runtime-for. - Use one attribute per runtime function to make merging easier. The patch is still WIP and needs more work. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

gchatelet added a comment. AFAIU here is a coarse plan of what needs to happen 1. Add a `no-builtin` clang function attribute that has the same semantic as the `no-builtin` cmd line argument 2. Propagate it

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

chandlerc added a comment. Sorry I've been a bit slow to respond here... In D61634#1503089 , @hfinkel wrote: > In D61634#1502201 , @tejohnson wrote: > > > In D61634#1502138

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

hfinkel added a subscriber: chandlerc. hfinkel added a comment. In D61634#1502201 , @tejohnson wrote: > In D61634#1502138 , @hfinkel wrote: > > > In D61634#1502043 ,

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

tejohnson added a comment. In D61634#1502685 , @gchatelet wrote: > In D61634#1502201 , @tejohnson wrote: > > > Using function level attributes instead of module flags does provide finer > > grained control and

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

gchatelet added a comment. In D61634#1502201 , @tejohnson wrote: > Using function level attributes instead of module flags does provide finer > grained control and avoids the conservativeness when merging IR for LTO. The > downsides I see, mostly just

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

tejohnson added a comment. In D61634#1502138 , @hfinkel wrote: > In D61634#1502043 , @efriedma wrote: > > > > I have a related patch that turns -fno-builtin* options into module flags > > > > Do you have any

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

hfinkel added a comment. In D61634#1502043 , @efriedma wrote: > > I have a related patch that turns -fno-builtin* options into module flags > > Do you have any opinion on representing -fno-builtin using a module flag vs. > a function attribute in IR? It

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

efriedma added a comment. > I have a related patch that turns -fno-builtin* options into module flags Do you have any opinion on representing -fno-builtin using a module flag vs. a function attribute in IR? It seems generally more flexible and easier to reason about a function attribute from

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

tejohnson added a comment. In D61634#1501066 , @gchatelet wrote: > In D61634#1500453 , @efriedma wrote: > > > My main blocker is that I want to make sure we're moving in the right > > direction: towards LLVM IR

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

gchatelet added a subscriber: tejohnson. gchatelet added a comment. In 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 >

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

efriedma added a comment. 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

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

gchatelet added a comment. In D61634#1498376 , @efriedma wrote: > I still think there are really two things you're trying to accomplish here, > which should be handled separately. > > 1. Add a function attribute that works like -fno-builtin-memcpy

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

efriedma added a comment. > -fno-builtin* is about preventing clang/llvm from recognizing that a piece of > code has the same semantic as a particular IR intrinsic, it has nothing to do > with preventing the compiler from generating runtime calls. It has a dual purpose for C library functions.

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

gchatelet added a comment. In D61634#1494518 , @alexandre.isoard wrote: > I'm not convinced by the approach. > > Can it still recognize the loop idiom into memcpy implementation but use > `memmove` (as only `memcpy` has been blacklisted)? Yes it can

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

gchatelet added a comment. In D61634#1493927 , @efriedma wrote: > I would be careful about trying to over-generalize here. There are a few > different related bits of functionality which seem to be interesting, given > the discussion in the llvm-dev

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

alexandre.isoard added a comment. I'm not convinced by the approach. Can it still recognize the loop idiom into memcpy implementation but use `memmove` (as only `memcpy` has been blacklisted)? Can it do the same for `memmove` (for the case without overlap), and end-up with infinite recursion?

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

efriedma added a comment. I would be careful about trying to over-generalize here. There are a few different related bits of functionality which seem to be interesting, given the discussion in the llvm-dev thread, here, and in related patches: 1. The ability to specify -fno-builtin* on a

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

gchatelet added a comment. In D61634#1493350 , @theraven wrote: > I wonder if a list of comma-separated names is the right approach. Would it > make more sense to add a new attribute for each of the helpers, such as > `#no-runtime-for-memcpy`? That

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

theraven added a comment. I wonder if a list of comma-separated names is the right approach. Would it make more sense to add a new attribute for each of the helpers, such as `#no-runtime-for-memcpy`? That should make querying simpler (one lookup in the table, rather than a lookup and a string

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

gchatelet updated this revision to Diff 198436. gchatelet added a comment. Herald added a subscriber: jdoerfert. - Add documentation. - Fix permissive HasNoRuntimeAttribute Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61634/new/

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

gchatelet created this revision. gchatelet added a reviewer: courbet. Herald added subscribers: llvm-commits, cfe-commits, mgrang, hiraditya. Herald added projects: clang, LLVM. POC for the rfc http://lists.llvm.org/pipermail/llvm-dev/2019-April/131973.html Repository: rG LLVM Github Monorepo