This is a follow up on https://reviews.llvm.org/D61634#1742154 to turn the clang driver -fno-builtin flag into an IR attribute.
I also investigated pushing the attribute earlier on (in Sema) but it looks like this patch is simple and will cover all function calls.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
- Build Status
Buildable 42281 Build 42718: arc lint + arc unit
Event Timeline
This LGTM, although I have a couple of questions that are orthogonal to this patch and shouldn't block it. Please wait to see if @aaron.ballman has any more comments.
clang/test/CodeGen/libcalls-fno-builtin.c | ||
---|---|---|
163 | Orthogonal to this patch: Looks like there are 2 nobuiltin attributes now? AFAICT the old "nobuiltin" gets applied to any and all cases where either -fno-builtin or -fno-builtin-{name} applied. Is it obviated by the new attributes? Also, why are the new ones quoted and the old one not? |
clang/test/CodeGen/libcalls-fno-builtin.c | ||
---|---|---|
163 | Here is my understanding so far:
Now I wanted to create a different attribute than the original nobuiltin because semantic might be different. Obviously this is not ideal.
For the individual ones the list is open ended so there's no way we can retrofit it into the previous scheme. @efriedma, @hfinkel do you have any suggestions or a better approach here? |
clang/test/CodeGen/libcalls-fno-builtin.c | ||
---|---|---|
163 | Thanks for the background. Yeah it seems like a good idea to remove the overlap here at some point, and the semantics do seem to be different at the moment. Like I said earlier, though, I don't think it should block this particular patch since the duplication predates it (and it also allows forward progress on the TLI side). |
clang/test/CodeGen/libcalls-fno-builtin.c | ||
---|---|---|
163 | SGTM I'll submit the patch then. |
You can drop the top-level const here (we don't typically top-level const qualify locals).