[RFC] NoBuiltin Attribute

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.

Having the attribute on the prototype of printf would tell us that calls to printf cannot be converted to calls to anything else. Isn't that what you want?

No, it isn't because that isn't sufficient. Besides the example I gave in the other email to this thread, consider things like:

void foo() {
  auto fp = printf;
  fp("xyz\n");
}

With -fno-builtin-printf, we can't optimize the call to printf, even though it only becomes apparent after (trivial) devirtualization.

-Chris

Yes, this is necessary.

I don’t understand why this is either necessary or sufficient.

Among other problems, doing this would requires us to:

  1. Add each of these to llvm.used (or add other hacks) to prevent them from being used.
  2. Add a ton of function prototypes when -fno-builtin is passed (since there are dozens or hundreds of different “builtin” functions that we optimize.
  3. Every time the optimizer learns to optimize some new libcall, we would have to change every llvm frontend that wants -fno-builtin to add the prototype.

Lets not do this! :slight_smile:

-Chris

Here are the cases that I see:
(1) Nobuiltin attribute on printf, telling the compiler that the printf that can appear in a call may not be the printf from libc, meaning that the compiler cannot do anything to any such call that would take advantage of knowing what printf would normally do.
(2) Nobuiltin attribute on some user function foo, telling the compiler that we don't want it to perform any optimizations related to the knowledge about builtins (or printf in particular, if it's nobuiltin-printf) in function foo.

Case (1) would imply (2), and not only for foo, but for all functions. On the other hand, (2) does not imply (1). In case (1) it may be considered an error to have conflicting attribute for printf in different compilation units, if the attribute is associated with the prototype of printf. However, it doesn't have to be an error if the attribute is actually present at every function call (instead of being attached to the prototype).

Case (2) would only have the attribute on calls that are originally in the body of foo. Otherwise we would run into trouble with inlining even without LTO.

This seems to agree with what you wrote (here and in other replies), and I suspect that the difference may be in some details of how we look at it.

I would suggest using separate compiler options to denote the two cases, something like:
(1) -fno-builtin-printf
(2) -fno-builtins-in-foo (or something of that nature).

There is also the problem of function overload in C++, templates, namespaces, etc. and I have no idea how to differentiate between different foos in a compiler option (without using mangled names). (This is not so much a problem for the real builtins, as they are mostly unmangled.)

-Krzysztof

What is the use case of this? This makes no sense for LTO, and minimal sense for the non-LTO case. What problem are you trying to solve here?

-Chris

Ha. Good example.
What would you expect to happen in this case?

--- a.cpp --- (with -fno-builtin-printf)
pointer fptr;
void foo(bool x) {
   bar(x);
}
void set(bool x) {
   if (x) fptr = printf;
   else fptr = vprintf;
}

Cases where users write their own replacements of standard functions. It's not legal from the standard's perspective, but it happens. As a matter of fact, this would be my primary goal here...

-Krzysztof

void foo() {
  auto fp = printf;
  fp("xyz\n");
}

With -fno-builtin-printf, we can't optimize the call to printf, even though it only becomes apparent after (trivial) devirtualization.

Ha. Good example.
What would you expect to happen in this case?

--- a.cpp --- (with -fno-builtin-printf)
pointer fptr;
void foo(bool x) {
bar(x);
}
void set(bool x) {
if (x) fptr = printf;
else fptr = vprintf;
}
-------------

--- b.cpp --- (no restrictions)
extern pointer fptr;
void bar(bool x) {
set(x);
va_list ap = ...
(*fptr)("haha", ap);
}
-------------

-fno-builtin applies to the code in foo and set.

Also, with the options reversed.

Then -fno-builtin would apply to the code in bar.

-Chris

How is this not handled by handling fno-builtin by putting an attribute on the code being compiled?

-Chris

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.

Ok, this makes a lot of sense to me.

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.

Sounds good, much of TLI can just disappear when this happens, woot.

I'm hoping the whole of TLI can disappear with the new attributes rewrite. :slight_smile:

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.

I guess I'm just asking "how much do we care" about that use case? Is it worth uglifying IR dumps?

There are two (old) bugs related to this: a PR and a radar. They both seem to be of low priority, though.

One thing that might help in this discussion is to note that attributes are *not* part of the function type (signature, whatever). So we cannot have a function declaration with 'nobuiltin' and another declaration without it.

Another thing to note is that during LTO I plan to restrict inlining of functions to those whose attributes match. This is an overly conservative approach, but one which will minimize generating code the user didn't want/expect. This restriction is meant to be relaxed over time on a case-by-case basis. In general though, I expect most TUs for a project will be compiled with the same command line flags, producing the same attributes for most functions.

So if `@foo' is marked `nobuiltin' and `@bar' isn't marked with `nobuiltin', then `@foo' will not be inlined into `@bar' and vice versa.

Here's what I propose then:

Note: I'm intentionally leaving out the `-fno-builtin-FUNCTION' flag, because clang doesn't yet support it, and that can be easily added later --- probably in the form I mentioned above.

I add the `NoBuiltin' attribute to the Attribute::AttrKind enum list and a `nobuiltin' keyword to the LLVM assembly language (documentation, etc.). This can be applied to function definitions, but not to function declarations or to call/invoke instructions, because that kind of granularity doesn't make a lot of sense to me. I want the attribute to be in the 'enum' list because string attributes are meant more for target-dependent attributes rather than IR-level attributes. (The distinction isn't set in stone, but I feel that this doesn't need to be a string.)

The LibSimplify code will check the function containing the call/invoke for the `NoBuiltin' attribute to see if it is allowed to simplify the builtin.

What do you think?

-bw

I guess I should have mentioned that I'm assuming link-time inlining...

-Krzysztof

Regardless of which solution you consider, it is always "putting an attribute on the code being compiled", the question is where exactly and what we do with it. I'm opposed to putting the attribute on the definitions of the callers of functions indicated with -fno-builtin-<something>.

If the user says -fno-builtin-printf, then all callers of printf would need to be marked as "nobuiltin". As your example demonstrates, we don't always know the exact set of callers of printf, since there can be indirect calls that may only later be resolved to direct calls.

Putting attributes on caller's bodies will pose difficulties during link-time inlining, which was also illustrated by your example from earlier in this branch of this thread.

If we implement it this way now, we may not be able to change it later, especially if we want to have backward compatibility of bitcode (i.e. future LLVMs compiling past bitcodes).

-Krzysztof

How is this not handled by handling fno-builtin by putting an attribute on the code being compiled?

Regardless of which solution you consider, it is always "putting an attribute on the code being compiled", the question is where exactly and what we do with it

No, it isn't. Putting it on IR prototypes is *not* putting the attribute on the code being compiled, it is putting it on something that is transient, that does not survive LTO merges, and that is aggressively deleted when unused. This is not the mechanism to build whatever feature it is that you think you're looking for.

. I'm opposed to putting the attribute on the definitions of the callers of functions indicated with -fno-builtin-<something>.

If the user says -fno-builtin-printf, then all callers of printf would need to be marked as "nobuiltin".

Yes, or "no-builtin=printf" if we bother to implement fine grained control.

As your example demonstrates, we don't always know the exact set of callers of printf, since there can be indirect calls that may only later be resolved to direct calls.

The attribute would also be put on all indirect calls: when/if they get devirtualized, exactly the right thing happens.

Putting attributes on caller's bodies will pose difficulties during link-time inlining, which was also illustrated by your example from earlier in this branch of this thread.

I don't think you understand how link-time inlining works.

If we implement it this way now, we may not be able to change it later, especially if we want to have backward compatibility of bitcode (i.e. future LLVMs compiling past bitcodes).

I still really have no idea what problem you think you are solving. Please give me a specific code example plus a series of steps to reproduce the issue you are imagining, exactly as I did up-thread.

Thanks!

-Chris

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.

Sounds good, much of TLI can just disappear when this happens, woot.

I'm hoping the whole of TLI can disappear with the new attributes rewrite. :slight_smile:

That would be nice, but I don't think that will happen 100%. We still need to know whether "memset pattern" exists, for example. I'm happy with it getting dramatically downsized :slight_smile:

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.

I guess I'm just asking "how much do we care" about that use case? Is it worth uglifying IR dumps?

There are two (old) bugs related to this: a PR and a radar. They both seem to be of low priority, though.

One thing that might help in this discussion is to note that attributes are *not* part of the function type (signature, whatever). So we cannot have a function declaration with 'nobuiltin' and another declaration without it.

You mean "So we *can* have a function declaration"...

Another thing to note is that during LTO I plan to restrict inlining of functions to those whose attributes match. This is an overly conservative approach, but one which will minimize generating code the user didn't want/expect.

This is exactly why these should be put on calls in the compiled scope, not on the function bodies being compiled, which I'm pretty sure your patch doesn't do.

Note: I'm intentionally leaving out the `-fno-builtin-FUNCTION' flag, because clang doesn't yet support it, and that can be easily added later --- probably in the form I mentioned above.

Right, sounds like a great second step if anyone ever cares enough to implement it.

I add the `NoBuiltin' attribute to the Attribute::AttrKind enum list and a `nobuiltin' keyword to the LLVM assembly language (documentation, etc.).

Sounds good.

This can be applied to function definitions, but not to function declarations or to call/invoke instructions, because that kind of granularity doesn't make a lot of sense to me.

I think we should do it, otherwise inlining is unnecessarily harmed. Given that attributes are displayed in .ll files with the # syntax, there should be no bloat/unreadability impact.

The LibSimplify code will check the function containing the call/invoke for the `NoBuiltin' attribute to see if it is allowed to simplify the builtin.
What do you think?

Sounds ok, but I think the attribute should be on compiled call sites instead of compiled function bodies. If we're going to do this, might as well do it right.

Thanks Bill,

-Chris

There are two (old) bugs related to this: a PR and a radar. They both seem to be of low priority, though.

One thing that might help in this discussion is to note that attributes are *not* part of the function type (signature, whatever). So we cannot have a function declaration with 'nobuiltin' and another declaration without it.

You mean "So we *can* have a function declaration"...

I'm confused. That's not something that works today. This doesn't assemble:

declare i32 @x() nounwind
declare i32 @x() noinline

define i32 @foo(i32 %a, i32 %b, i32 %c) nounwind {
    %t0 = tail call i32 @x() nounwind
    %t1 = sdiv i32 %a, %b
    ret i32 %t1
}

Do you want this to change?

Another thing to note is that during LTO I plan to restrict inlining of functions to those whose attributes match. This is an overly conservative approach, but one which will minimize generating code the user didn't want/expect.

This is exactly why these should be put on calls in the compiled scope, not on the function bodies being compiled, which I'm pretty sure your patch doesn't do.

Okay. I was confused by the previous discussion then.

This can be applied to function definitions, but not to function declarations or to call/invoke instructions, because that kind of granularity doesn't make a lot of sense to me.

I think we should do it, otherwise inlining is unnecessarily harmed. Given that attributes are displayed in .ll files with the # syntax, there should be no bloat/unreadability impact.

The LibSimplify code will check the function containing the call/invoke for the `NoBuiltin' attribute to see if it is allowed to simplify the builtin.
What do you think?

Sounds ok, but I think the attribute should be on compiled call sites instead of compiled function bodies. If we're going to do this, might as well do it right.

Alright, then the attribute will be applied to the call/invoke instructions and not the function definitions and declarations.

-bw

Dealing with different attributes on different functions.

--- a.c ---
void func_a() {
   printf(...);
}

--- b.c ---
void func_b() {
   printf(...);
   func_a();
}

a.c is compiled with no-builtin-printf, b.c has no such options.

The prototype approach (no-builtin on the prototype of printf) won't work in case like this, when the no-builtin attributes are not identical across all the compilation units.

Putting this attribute on all calls to printf (and all indirect calls) in a.c should work fine.

Putting the attribute on the caller of printf, i.e. "func_a" is what I object to. If it's in the form of "define @func_a() no-builtin-printf {...body...}", then the attribute may be lost during inlining of func_a. If this is solved by transferring the attribute to the calls, then there is no need for any specific association of the attribute with func_a, since it can be placed on the calls from the beginning.

-Krzysztof

I still really have no idea what problem you think you are solving.

Dealing with different attributes on different functions.

— a.c —
void func_a() {
printf(…);
}

— b.c —
void func_b() {
printf(…);
func_a();
}

a.c is compiled with no-builtin-printf, b.c has no such options.

The prototype approach (no-builtin on the prototype of printf) won’t work in case like this, when the no-builtin attributes are not identical across all the compilation units.

Putting this attribute on all calls to printf (and all indirect calls) in a.c should work fine.

I’m still not understanding a few things in this thread, including one here: if you annotate only the calls to print (say) then how do you handle the indirect calls that the back end might yet optimize down to a constant & then attempt to simplify? Would all indirect calls be annotated with all the unsimplifiable function names?

Putting the attribute on the caller of printf, i.e. “func_a” is what I object to. If it’s in the form of “define @func_a() no-builtin-printf {…body…}”, then the attribute may be lost during inlining of func_a.

I believe Bill had suggested solving this in the first pass by not inlining incompatible attributes.

If this is solved by transferring the attribute to the calls, then there is no need for any specific association of the attribute with func_a, since it can be placed on the calls from the beginning.

I’m not quite sure what you mean by ‘can be placed on calls from the beginning’. One of the core issues here is LTO where two bit code files were compiled with different options and then linked together - then one function from one bitcode is inclined into a function from the other - in this case there was no opportunity to have marked the latter with the right attribute ahead of time.

(And somehow I ended up adding a random non functioning email alias to the ‘to’ line in my last reply, removed in this reply. Sorry about that)

I'm still not understanding a few things in this thread, including one
here: if you annotate only the calls to print (say) then how do you
handle the indirect calls that the back end might yet optimize down to a
constant & then attempt to simplify? Would all indirect calls be
annotated with all the unsimplifiable function names?

Something like that. Indirect calls could be conservatively marked as "no-builtin-everything", to limit the amount of data attached to them. Metadata could be used to indicate exactly which target functions were of interest, but it could be ignored without violating the original options.

I'm not quite sure what you mean by 'can be placed on calls from the
beginning'. One of the core issues here is LTO where two bit code files
were compiled with different options and then linked together - then one
function from one bitcode is inclined into a function from the other -
in this case there was no opportunity to have marked the latter with the
right attribute ahead of time.

I don't think it should be marked with anything. Maybe this is the root cause of this ongoing misunderstanding. If the user compiles one function with no-builtin-printf, then only the calls to printf that were originally in this function should be subjected to this restriction. If this function is then inlined into another function that didn't have any no-builtin attributes, then calls to printf originating from that other functions are not restricted. This way you can end up with two calls to printf in the same function, one of them restricted, the other one unrestricted.

-Krzysztof

I’m still not understanding a few things in this thread, including one
here: if you annotate only the calls to print (say) then how do you
handle the indirect calls that the back end might yet optimize down to a
constant & then attempt to simplify? Would all indirect calls be
annotated with all the unsimplifiable function names?

Something like that. Indirect calls could be conservatively marked as “no-builtin-everything”, to limit the amount of data attached to them. Metadata could be used to indicate exactly which target functions were of interest, but it could be ignored without violating the original options.

I’m not quite sure what you mean by ‘can be placed on calls from the
beginning’. One of the core issues here is LTO where two bit code files
were compiled with different options and then linked together - then one
function from one bitcode is inclined into a function from the other -
in this case there was no opportunity to have marked the latter with the
right attribute ahead of time.

I don’t think it should be marked with anything. Maybe this is the root cause of this ongoing misunderstanding. If the user compiles one function with no-builtin-printf, then only the calls to printf that were originally in this function should be subjected to this restriction. If this function is then inlined into another function that didn’t have any no-builtin attributes, then calls to printf originating from that other functions are not restricted. This way you can end up with two calls to printf in the same function, one of them restricted, the other one unrestricted.

Sure, if you’re willing to sacrifice the possible simplification of all indirect calls in any function that has even one nobuiltin requirement.

  1. annotate calls
    Pro: you can inline calls without pessimizing the function you inline into
    Con: you pessimize all indirect calls in functions with at least one nobuiltin requirement

  2. annotate functions
    Pro: you don’t penalize indirect calls
    Con: you either penalize functions that are inlined into, or you avoid inlining.

Now, surem , I would guess the number of indirect calls that end up being simplified is probably fairly small (though I don’t really have a clue) which would err in favor of (1), but I just wanted to be clear what we’re trading off.

Were there any other options being considered or benefits/drawbacks of these options?

2) annotate functions
Pro: you don't penalize indirect calls
Con: you either penalize functions that are inlined into, or you avoid
inlining.

Another problem here is that if foo is marked as no-builtin-printf, then any function inlined into foo would "inherit" that attribute. So, the inlining penalty works on both sides---whoever inlines it gets it, and whoever is inlined into it gets it.

Were there any other options being considered or benefits/drawbacks of
these options?

My original thought was to associate the no-builtin attribute with the prototype of the builtin that it not to be optimized. The problem with that was that it wouldn't work well with LTO in cases where different CUs had different options. In other words, movement of code across such CUs would be very restricted (or completely prohibited), if we don't want to pessimize the attributes to the "common denominator". On the other hand, prototypes have this advantage that they can provide centralized information about a specific function without having a definition of that function. Indirect call instructions would not be affected by the no-builtin attribute since if (after resolving the indirect call) the target function turns out to be printf, a lookup for the attribute on the printf's prototype would tell us that it's a "no-builtin".

-Krzysztof