On Feb 18, 2013, at 4:45 PM, Chris Lattner <clattner at apple.com> wrote:> > On Feb 18, 2013, at 1:54 PM, Bill Wendling <wendling at apple.com> wrote: > >> Hi LLVMites! >> >> This patch adds the 'nobuiltin' attribute to to LLVM. This is needed during LTO, which right now ignores this attribute and command line flag. I want to make this an IR-level attribute instead of a target-dependent attribute because it's used during IR modification and not code generation. >> > > Hi Bill, > > I think the concept of this patch makes sense, but the implementation does not. > > I have: > > void foo() { > printf("hello\n"); > } > > and I build with -fno-builtin-puts. If I understand correctly, *foo* will be marked with "nobuiltin", but this code in simplifylibcalls looks at the printf: > > Value *LibCallSimplifier::optimizeCall(CallInst *CI) { > + Function *F = CI->getCalledFunction(); > + if (F->hasFnAttribute(Attribute::NoBuiltin)) return 0; > return Impl->optimizeCall(CI); > } > > In the context of LTO, it makes sense for the attribute to be on function bodies, not on prototypes. >Yeah, I noticed that after sending this patch. I modified it to check the function CI is in for that attribute. Once we have support for the `-fno-builtin-FUNCTION' flag, I expect the attribute to look something like this: "no-builtin-functions" = "puts,foo,bar" -bw
On Mon, Feb 18, 2013 at 4:49 PM, Bill Wendling <wendling at apple.com> wrote:> On Feb 18, 2013, at 4:45 PM, Chris Lattner <clattner at apple.com> wrote: > > > > > On Feb 18, 2013, at 1:54 PM, Bill Wendling <wendling at apple.com> wrote: > > > >> Hi LLVMites! > >> > >> This patch adds the 'nobuiltin' attribute to to LLVM. This is needed > during LTO, which right now ignores this attribute and command line flag. I > want to make this an IR-level attribute instead of a target-dependent > attribute because it's used during IR modification and not code generation. > >> > > > > Hi Bill, > > > > I think the concept of this patch makes sense, but the implementation > does not. > > > > I have: > > > > void foo() { > > printf("hello\n"); > > } > > > > and I build with -fno-builtin-puts. If I understand correctly, *foo* > will be marked with "nobuiltin", but this code in simplifylibcalls looks at > the printf: > > > > Value *LibCallSimplifier::optimizeCall(CallInst *CI) { > > + Function *F = CI->getCalledFunction(); > > + if (F->hasFnAttribute(Attribute::NoBuiltin)) return 0; > > return Impl->optimizeCall(CI); > > } > > > > In the context of LTO, it makes sense for the attribute to be on > function bodies, not on prototypes. > > > Yeah, I noticed that after sending this patch. I modified it to check the > function CI is in for that attribute. Once we have support for the > `-fno-builtin-FUNCTION' flag, I expect the attribute to look something like > this: > > "no-builtin-functions" = "puts,foo,bar" >I wonder... why not attach the no-builtin attribute to the call instruction? It seems like that would avoid the need of a string attribute altogether, having the frontend match up calls to specific functions which should not be considered calls to builtin functions? For the general case of just '-ffreestanding' or whichever, you might do something different if annotating every call instruction is prohibitively expensive. But I've not thought about this deeply, I just find the idea of having the backend be responsible for matching up the function names somewhat distasteful. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130218/e577857f/attachment.html>
On Feb 18, 2013, at 5:01 PM, Chandler Carruth <chandlerc at google.com> wrote:> > In the context of LTO, it makes sense for the attribute to be on function bodies, not on prototypes. > > > Yeah, I noticed that after sending this patch. I modified it to check the function CI is in for that attribute. Once we have support for the `-fno-builtin-FUNCTION' flag, I expect the attribute to look something like this: > > "no-builtin-functions" = "puts,foo,bar" > > I wonder... why not attach the no-builtin attribute to the call instruction? It seems like that would avoid the need of a string attribute altogether, having the frontend match up calls to specific functions which should not be considered calls to builtin functions?How does this work? If I build with -fno-builtin-fputs, why would an attribute be added to a call to printf? -Chris -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130218/372556b0/attachment.html>
On Feb 18, 2013, at 4:49 PM, Bill Wendling <wendling at apple.com> wrote:>> Hi Bill, >> >> I think the concept of this patch makes sense, but the implementation does not. >> >> I have: >> >> void foo() { >> printf("hello\n"); >> } >> >> and I build with -fno-builtin-puts. If I understand correctly, *foo* will be marked with "nobuiltin", but this code in simplifylibcalls looks at the printf: >> >> Value *LibCallSimplifier::optimizeCall(CallInst *CI) { >> + Function *F = CI->getCalledFunction(); >> + if (F->hasFnAttribute(Attribute::NoBuiltin)) return 0; >> return Impl->optimizeCall(CI); >> } >> >> In the context of LTO, it makes sense for the attribute to be on function bodies, not on prototypes. >> > Yeah, I noticed that after sending this patch. I modified it to check the function CI is in for that attribute.Was that in the patch you committed? What specific patch are you looking for?> Once we have support for the `-fno-builtin-FUNCTION' flag, I expect the attribute to look something like this: > > "no-builtin-functions" = "puts,foo,bar"I guess this could work, this means that simplifylibcalls (and others) would check this instead of TargetLibraryInfo? Out of curiosity, how important is -fno-builtin-foo? Isn't it enough to just have -fno-builtin-foo disable *all* builtin optimizations for the unit of code being compiled? This is overly conservative - is that actually a problem for something? -Chris
On Mon, Feb 18, 2013 at 06:10:48PM -0800, Chris Lattner wrote:> Out of curiosity, how important is -fno-builtin-foo?Depends, does that include alloca and the object size stuff?> Isn't it enough to just have -fno-builtin-foo disable *all* builtin > optimizations for the unit of code being compiled? This is overly > conservative - is that actually a problem for something?Making -fno-builtin-foo imply -fno-builtin seems overly aggressively. I can imagine wanting to use the former when implementing libc to get more predictable behavior, but still wanting to get e.g. the string optimisations. As I mentioned on IRC, there is also the related issue of explicitly requesting a builtin, i.e. a positive __attribute__((__builtin__)) for freestanding code. I don't think that -fno-builtin-puts should imply -fno-builtin-printf. It should apply to function calls in this module, but may be merged by adding the nobuiltin attribute to the prototypes in all other modules during LTO. Joerg
On Feb 18, 2013, at 6:10 PM, Chris Lattner <clattner at apple.com> wrote:> On Feb 18, 2013, at 4:49 PM, Bill Wendling <wendling at apple.com> wrote: >>> Hi Bill, >>> >>> I think the concept of this patch makes sense, but the implementation does not. >>> >>> I have: >>> >>> void foo() { >>> printf("hello\n"); >>> } >>> >>> and I build with -fno-builtin-puts. If I understand correctly, *foo* will be marked with "nobuiltin", but this code in simplifylibcalls looks at the printf: >>> >>> Value *LibCallSimplifier::optimizeCall(CallInst *CI) { >>> + Function *F = CI->getCalledFunction(); >>> + if (F->hasFnAttribute(Attribute::NoBuiltin)) return 0; >>> return Impl->optimizeCall(CI); >>> } >>> >>> In the context of LTO, it makes sense for the attribute to be on function bodies, not on prototypes. >>> >> Yeah, I noticed that after sending this patch. I modified it to check the function CI is in for that attribute. > > Was that in the patch you committed? What specific patch are you looking for? >Yeah, that was in the one that I committed. I basically want something like this: define void @foo() "no-builtin" { call void @printf() } And then the `call' to printf would check for the "no-builtin" attribute on `@foo' to determine if it can convert it to `puts'. Having the attribute on the declaration of `printf' doesn't help us here. And I'm not sure if having it on the call would be beneficial either.>> Once we have support for the `-fno-builtin-FUNCTION' flag, I expect the attribute to look something like this: >> >> "no-builtin-functions" = "puts,foo,bar" > > I guess this could work, this means that simplifylibcalls (and others) would check this instead of TargetLibraryInfo? >Yes.> Out of curiosity, how important is -fno-builtin-foo? Isn't it enough to just have -fno-builtin-foo disable *all* builtin optimizations for the unit of code being compiled? This is overly conservative - is that actually a problem for something? >Being overly conservative will of course keep the correct semantics for the program. I suppose that there could be some people who have their own special version of a builtin that they want to use, but they don't want all builtin optimizations to be disabled. -bw