[clang][IR] Add support for leaf attribute

[gulfem]

Summary

GCC supports attribute((leaf)) as an optimization hint described in:
https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html
This patch adds support for this attribute in Clang/LLVM.

[jdoerfert]

Tests missing (on all levels), was there an RFC? What is the expected usage of this?

[jdoerfert]

(To clarify, I am not opposing this, my comment before was brief but not meant to be negative. I'm generally for new attributes and this one is already in existing source so that is even better.)

[mcgrathr]

There is an RFC going out with this prototype as reference. When there is consensus on the RFC, this will get in shape for landing with complete tests and all.

[jdoerfert]

In D90275#2357931, @mcgrathr wrote:

There is an RFC going out with this prototype as reference. When there is consensus on the RFC, this will get in shape for landing with complete tests and all.

Great. I didn't find anything when I looked for leaf in my cfe-dev inbox. Could someone put a link here, maybe directly into the commit message as it will be useful even after this goes in. Thanks :)

[gulfem]

In D90275#2357945, @jdoerfert wrote:

In D90275#2357931, @mcgrathr wrote:

There is an RFC going out with this prototype as reference. When there is consensus on the RFC, this will get in shape for landing with complete tests and all.

Great. I didn't find anything when I looked for leaf in my cfe-dev inbox. Could someone put a link here, maybe directly into the commit message as it will be useful even after this goes in. Thanks :)

I just sent the RFC with the subject line [RFC] Leaf Attribute Support in Clang/LLVM.

[jdoerfert]

In D90275#2357951, @gulfem wrote:

In D90275#2357945, @jdoerfert wrote:

In D90275#2357931, @mcgrathr wrote:

There is an RFC going out with this prototype as reference. When there is consensus on the RFC, this will get in shape for landing with complete tests and all.

Great. I didn't find anything when I looked for leaf in my cfe-dev inbox. Could someone put a link here, maybe directly into the commit message as it will be useful even after this goes in. Thanks :)

I just sent the RFC.

Yep, I'm writing my response there ;)

FWIW, http://lists.llvm.org/pipermail/llvm-dev/2020-October/146113.html

[leonardchan]

I'd recommend splitting this into two patches, one for LLVM changes and another for Clang changes.

[gulfem]

Add IR and bitcode tests

[gulfem]

Fix the typo in the test case

[aaron.ballman]

clang/include/clang/Basic/Attr.td

1453

Should this attribute also be supported on things like ObjC method decls or other function-like interfaces?

1454

This should be referring to LeafDocs rather than undocumented.

clang/include/clang/Basic/AttrDocs.td

3984

as leaf -> with the leaf

I'm not certain how to interpret that functions are not allowed to enter their caller's translation unit. I sort of read that as leaf functions are not allowed to call (or otherwise jump) out of the translation unit in which they're defined -- is that about right?

3985

What sort of diagnostic checking should the user expect? e.g., can the user expect Clang to diagnose obviously incorrect code like:

[[gnu::leaf]] void func(void (*fp)(void)) {
  fp(); // This seems like a bad thing
}

3988

We should probably link to the latest docs (https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html) rather than 4.7.2.

[gulfem]

Addressed the comments about the documentation

[gulfem]

clang/include/clang/Basic/AttrDocs.td

3984

I think the description is a little confusing.
As far as I understand, leaf functions can actually call or jump out the the translation unit that they are defined ("Leaf functions might still call functions from other compilation units").
The manual refers caller function's translation unit as current translation unit.
"Calls to external functions with this attribute must return to the current compilation unit only by return or by exception handling. In particular, a leaf function is not allowed to invoke callback functions passed to it from the current compilation unit, directly call functions exported by the unit, or longjmp into the unit."
My interpretation of this statement is that a function marked with a leaf attribute can only return to its caller translation unit by a return or an exception, but it cannot enter into callers translation unit by invoking a callback function.
Does that make sense?

[aaron.ballman]

clang/include/clang/Basic/AttrDocs.td

3984

Ah, thank you for the explanation! How about: Functions marked with the ''leaf'' attribute are not allowed to jump back into the caller's translation unit, whether through invoking a callback function, a direct external function call, use of 'longjmp', or other means.?

Is this property transitive? e.g.,

// TU1.c
void func(void) {
  leaf();
}

void bar(void) {}

// TU2.c
__attribute__((leaf)) void leaf(void) {
  baz(); // Is this not allowed?
}

// TU3.c
void baz(void) {
  bar();
}

The "directly call functions exported by the unit" makes me think the above code is fine but it could also be that "direct" in this case means "through a function designator rather than a function pointer".

[gulfem]

clang/include/clang/Basic/Attr.td

1453

Do I need to do anything else to support this attribute in Objective-C as well?
I think we should support it in all the C languages family.

[aaron.ballman]

clang/include/clang/Basic/Attr.td

1453

I think we should support it in all the C languages family.

That's already happening automatically -- there's a C and C++ spelling available for it and the attribute doesn't specify that it requires a particular language mode or target.

Do I need to do anything else to support this attribute in Objective-C as well?

You can add multiple subjects to the list here, so you can have this apply to Function, ObjCMethod for both of those. Another one to consider is whether this attribute can be written on a block declaration (like a lambda, but with different syntax). Beyond that, it's mostly just documentation, devising the test cases to ensure the ObjC functionality behaves as expected, possibly some codegen changes, etc.

[gulfem]

Update the attribute documentation

[gulfem]

clang/include/clang/Basic/AttrDocs.td

3985

This is a compiler hint provided by the user, and if the user misuses that attribute, it might result in undefined behavior.
I think it might be difficult to fully verify that leaf function does not enter into caller's translation unit.
For this simple case you provided, it might be easy to add such a check.
However, it might be difficult to add such checks for complicated cases.
Therefore, we were not planning to add such kind of diagnostic checking.
What do you think?

3985

I think this property is transitive.
If a leaf function somehow enters into caller's translation unit (either via direct call or a via its call chain), it will violate the rule that says "Calls to external functions with this attribute must return to the current compilation unit only by return or by exception handling".
Entering via its call chain is not a return or an exception handling.
Do you agree with that?

[jdoerfert]

clang/include/clang/Basic/AttrDocs.td

3985

@aaron.ballman, your code example is not " obviously incorrect " and we should not diagnose that (see below). In fact, I doubt we can ever diagnose misuse except during the LTO linking stage, assuming the expected use case. That said, you could warn and ignore if a function definition is annotated.

// library.c
void bar(void) {}

[[gnu::leaf]] void func(void (*fp)(void)) {
  fp(); // not necessarily a bad thing
}
// user.c
#include <library.h>
void test() {
  func(&bar); // perfectly fine.
}

[gulfem]

clang/include/clang/Basic/AttrDocs.td

3984

I like your description, and I changed the doc that incorporates your feedback.

[aaron.ballman]

clang/include/clang/Basic/AttrDocs.td

3985

@aaron.ballman, your code example is not " obviously incorrect " and we should not diagnose that (see below).

Oh, thank you for that example!

That said, you could warn and ignore if a function definition is annotated.

Why would you want to warn and ignore in that situation?

3985

I think this property is transitive. ... Do you agree with that?

That makes sense to me! I think I'd recommend a slight modification to the docs then:

Functions marked with the ``leaf`` attribute are not allowed to jump back into the caller's translation unit, whether through invoking a callback function, a direct, possibly transitive, external function call, use of ``longjmp``, or other means.

The last question is: by "direct" does the GCC docs imply that calls back to the caller's TU through a function *pointer* are not UB? I'd imagine those would also be bad, but I'd like to make sure (and perhaps we can remove the direct from the wording if calls through function pointers are disallowed).

[jdoerfert]

The more I think about it, the more I think we should never create a leaf/nocallback definition. Only declarations should carry that attribute.

I'm also still not convinced nocallback is a good name. @efriedma @aqjune @fhahn @reames What are your thoughts on the name. My earlier idea was inaccesiblecodeonly, as it matches the inaccisblememonly idea. I'm not married to it, nocallback is just not great IMHO as direct calls back into the caller TU are also forbidden and there is already !callback.

clang/include/clang/Basic/AttrDocs.td

3985

That said, you could warn and ignore if a function definition is annotated.

Why would you want to warn and ignore in that situation?

Let me try to explain, sorry I didn't earlier.

GCC says (emphasis added):

Calls to *external* functions with this attribute
...
The attribute has *no effect on functions defined* within the current compilation unit. This is to allow easy merging of multiple compilation units into one, for example, by using the link-time optimization. For this reason the attribute is not allowed on types to annotate indirect calls.

The thing is, it doesn't make sense to have a leaf definition. Leaf functions can always call stuff "in their own TU".
Let's assume we have a leaf function definition in a TU and it is a single TU, all functions can be (=might be allowed to be) called by the leaf function, even external ones.
If it is a TU linked from multiple ones, we don't know anymore what came from where so there is no point in keeping leaf on a definition as the call sites all look the same.

[aaron.ballman]

In D90275#2371343, @jdoerfert wrote:

The more I think about it, the more I think we should never create a leaf/nocallback definition. Only declarations should carry that attribute.

When talking about the IR that gets lowered to LLVM, I think that's very reasonable. When talking about the frontend, I'm a bit less certain. GCC and ICC both do not warn when you apply the attribute solely to a definition, so one concern is with code portability where existing code is harder to port into Clang. However, when I tried to find examples of the attribute being written on a definition, I couldn't spot any so this may not be a particularly large concern.

Another potential concern is that a situation like this seems harmless and diagnosing the use on the definition seems user-unfriendly:

// In a header file
__attribute__((leaf)) void func(void);

// In a source file
__attribute__((leaf)) void func(void) { ... }

The last concern is whether non-compiler-writing users will understand the behavior.

// #1
__attribute__((leaf)) void func(void) {} // Declares and defines a function at the same time

// #2
__attribute__((leaf)) void func(void);
void func(void) {} // Inherits the attribute from the previous declaration

I would suspect that users will see #1 and #2 as defining the same things and may be surprised if #1 is diagnosed but #2 is fine, but that's just a gut feeling without evidence backing it up.

I'm not insisting that we accept the attribute on a definition, but if we do want to make a distinction in the frontend between declaration and definition, we should document it with some explicit examples.

[aqjune]

Hi,

Naming is a hard thing... I have no special preference. :/

However, I'd like to understand the details of this attribute.

Would LTO be affected because leaf is guaranteed to untouch the current translation unit only?

// a.c
int x;
void f1() {
  f2();
}
void g() { x = 3; }

// b.c
void f2() {
  leaf();
}

// leaf.c
attribute((leaf)) void leaf() {
  g();
}

IIUC this program is okay because g() and the caller of leaf() are in different TUs.
But, let's assume that a.c and b.c are LTO-ed, and leaf.c is separately compiled.
If LTO merges a.c and b.c into the same module, the two TUs cannot be distinguished anymore; either leaf should be dropped, or LTO should somehow conceptually keep two TUs.
Would it be a valid concern? Then I think it should be mentioned.

Another question is more about the motivation of this attribute (well, I know it is introduced by gcc first; just throwing a question :) )
If the motivation is to support better data flow analysis, is there something special in callback itself?
The gcc document states that sin() is a leaf function, and IIUC this is because sin() never touches the memory allocated at caller's TU (because errno isn't at the caller's TU).
I think things are easier if we simply say that leaf cannot touch the memory of current TU, regardless of the existence of callbacks.
Is there something important in the callback itself?

[jdoerfert]

In D90275#2371764, @aqjune wrote:

Hi,

Naming is a hard thing... I have no special preference. :/

However, I'd like to understand the details of this attribute.

Would LTO be affected because leaf is guaranteed to untouch the current translation unit only?

// a.c
int x;
void f1() {
  f2();
}
void g() { x = 3; }

// b.c
void f2() {
  leaf();
}

// leaf.c
attribute((leaf)) void leaf() {
  g();
}

IIUC this program is okay because g() and the caller of leaf() are in different TUs.
But, let's assume that a.c and b.c are LTO-ed, and leaf.c is separately compiled.
If LTO merges a.c and b.c into the same module, the two TUs cannot be distinguished anymore; either leaf should be dropped, or LTO should somehow conceptually keep two TUs.
Would it be a valid concern? Then I think it should be mentioned.

As noted by the GCC docs, it doesn't mean anything on a definition so that you can safely merge TUs. I want us to forbid leaf on IR function definitions for that reason, it would not mean anything and be only confusing.

Another question is more about the motivation of this attribute (well, I know it is introduced by gcc first; just throwing a question :) )
If the motivation is to support better data flow analysis, is there something special in callback itself?
The gcc document states that sin() is a leaf function, and IIUC this is because sin() never touches the memory allocated at caller's TU (because errno isn't at the caller's TU).

No, that is not it. It is leaf because it will not transfer control to the callers TU.

I think things are easier if we simply say that leaf cannot touch the memory of current TU, regardless of the existence of callbacks.
Is there something important in the callback itself?

Not really, IMHO, leaf means you cannot transfer control to the callers TU without going threw the original call site (via return or throw).
It is not a memory thing. However, the "almost" matching memory property is called inaccesiblememonly so that is why I wanted to call this inaccessiblecodeonly.

[gulfem]

In D90275#2371343, @jdoerfert wrote:

The more I think about it, the more I think we should never create a leaf/nocallback definition. Only declarations should carry that attribute.

I'm also still not convinced nocallback is a good name. @efriedma @aqjune @fhahn @reames What are your thoughts on the name. My earlier idea was inaccesiblecodeonly, as it matches the inaccisblememonly idea. I'm not married to it, nocallback is just not great IMHO as direct calls back into the caller TU are also forbidden and there is already !callback.

We are totally fine to rename it to something else, and we do see the potential confusion with the existing callback attribute that we have.
If I understand correctly, inaccessiblememonly functions may only access memory that is not accessible by the current compilation unit (current compilation unit means the compilation unit that inaccessiblememonly function is defined).
If we use inaccesiblecodeonly, it implies that leaf functions may only access code that is not accessible by the compilation unit that they are defined.
But, leaf attribute does not enforce any rule for accessing code in the leaf attribute function's compilation.
It only enforces that leaf function does not access any code in its caller's translation unit.
That's is why I'm not sure whether inaccesiblecodeonly is a good name for that.
Please let me know if I'm missing anything about inaccessiblememonly attribute.
@jdoerfert what do you think about that?

[gulfem]

Add a target into the test case

[gulfem]

clang/include/clang/Basic/AttrDocs.td

3985

I think the idea is that the control-flow should never come back to the caller's translation unit by any kind of control-flow changing mechanism (direct/indirect and call/jump).
The compiler might use this hint to do optimizations based on that assumption, so the user should ensure that.
Could you please explain what do you mean by calling back via function pointer?
I thought that callback function is basically calling back via a function pointer.

[mcgrathr]

The GCC documentation specifically gives the example of the standard C function qsort as one that does not qualify as __attribute__((leaf)) because it uses a callback function (that presumably might be from the caller's own TU). AIUI the claim the attribute makes is that no normal control flow (i.e. excluding only signal handlers) would call any function in the caller's TU by any means, nor longjmp to any state captured by a setjmp call in the caller's TU, and thus only normal return or normal C++ exception handling return/unwind would reach the caller's TU again after the jump (call) to this entry point. The caller is thus presumed not to share potentially-called functions with the callee by any means, whether direct symbol references or function pointers passed in the call or function pointers previously stored somewhere (e.g. a C++ vtable). The compiler can make whatever assumptions about potential dataflow/side-effect and the like are implied by this exclusion of TU-local code paths from the otherwise unknown call graph of the external call.

[aqjune]

In D90275#2371813, @jdoerfert wrote:

As noted by the GCC docs, it doesn't mean anything on a definition so that you can safely merge TUs. I want us to forbid leaf on IR function definitions for that reason, it would not mean anything and be only confusing.

Okay, I see.
I agree that having this attribute at definitions is slightly dangerous..!

But I am not still 100% sure about the safety of merging... I see that it is okay when leaf is on a definition, but what about declaration?

// a.ll
define void f1() { f2(); }
define void g() { x = 3; }
// b.ll
define void f2() { leaf(); }
declare leaf void @leaf() ; If @leaf() was actually calling @g(), is merging a.ll and b.ll valid?

It is not a memory thing. However, the "almost" matching memory property is called inaccesiblememonly so that is why I wanted to call this inaccessiblecodeonly.

Eh, actually my question was more like a question to gcc people about why leaf was defined that way. Maybe my question is not relevant in this thread. Thank you for answering though.

[gulfem]

clang/include/clang/Basic/Attr.td

1453

AFAIK, users can specify function attributes in lambda expressions.
Lambda functions can only be accessed/called by the functions in the same translation unit, right?
Leaf attribute does not have any effect on the functions that are defined in the same translation unit.
For this reason, I'm thinking that leaf attribute would not have any effect if they are used in lambda expressions.
Do you agree with me?

[gulfem]

In D90275#2371343, @jdoerfert wrote:

The more I think about it, the more I think we should never create a leaf/nocallback definition. Only declarations should carry that attribute.

Leaf attribute is specifically intended for library functions and I think all the existing usage of leaf attribute is in the library function declarations.
For ex, it is only used in syscalls in Fuchsia.
Therefore, I'm not sure whether it is really necessary to ban leaf attribute in function definitions.
Even though function attributes are typically intended to be used in the function declaration, compilers do not have policy to forbid using them in the function definition.

[aqjune]

In D90275#2374648, @gulfem wrote:

Leaf attribute is specifically intended for library functions and I think all the existing usage of leaf attribute is in the library function declarations.
For ex, it is only used in syscalls in Fuchsia.
Therefore, I'm not sure whether it is really necessary to ban leaf attribute in function definitions.
Even though function attributes are typically intended to be used in the function declaration, compilers do not have policy to forbid using them in the function definition.

Maybe it's because the leaf (or nocallback or whatever) attribute at the function definition in IR can't be used for any optimization?
For the callers, leaf attribute at the function declarations is already giving the information.
But the attribute at function definitions really give no additional information inside the TU, so it's redundant.

[aaron.ballman]

clang/include/clang/Basic/Attr.td

1453

AFAIK, users can specify function attributes in lambda expressions.

I always forget that you can do that for declaration attributes using GNU-style syntax...

Lambda functions can only be accessed/called by the functions in the same translation unit, right?

Not necessarily, you could pass one across TU boundaries like a function pointer, for instance. e.g.,

// TU1.cpp
void foo() {
  auto l = []() { ... };
  bar(l);
}

// TU2.cpp
void bar(auto func) {
  func();
}

clang/include/clang/Basic/AttrDocs.td

3985

Could you please explain what do you mean by calling back via function pointer?
I thought that callback function is basically calling back via a function pointer.

I had confused myself -- I think @jdoerfert answered my confusion in an earlier comment and I just wasn't thinking about that case hard enough. I think we want to drop the word "direct" from "external function call" because it doesn't matter whether the user says:

void bar(void); // Assume calling this is bad
bar();

and

void bar(void); // Assume calling this is bad
void (*fp)(void) = bar;
fp();

[gulfem]

Remove direct from description and ObjCMethod for Obj functionality

[gulfem]

clang/include/clang/Basic/Attr.td

1453

Not necessarily, you could pass one across TU boundaries like a function pointer, for instance. e.g.,

As I mentioned before, leaf attribute is specifically intended for library functions and I think all the existing usage of leaf attribute is in the library function declarations. For this reason, I think we do not need to support them for lambdas. Is that reasonable?

clang/include/clang/Basic/AttrDocs.td

3985

Could you please explain what do you mean by calling back via function pointer?
I thought that callback function is basically calling back via a function pointer.

I had confused myself -- I think @jdoerfert answered my confusion in an earlier comment and I just wasn't thinking about that case hard enough. I think we want to drop the word "direct" from "external function call" because it doesn't matter whether the user says:

void bar(void); // Assume calling this is bad
bar();

and

void bar(void); // Assume calling this is bad
void (*fp)(void) = bar;
fp();

3985

I removed the direct from the documentation.

[aaron.ballman]

clang/include/clang/Basic/Attr.td

1453

For this reason, I think we do not need to support them for lambdas. Is that reasonable?

Is this considered a library function?

struct S {
  void f(); // Is this a library function?
  void operator()(); // How about this?
};

If the answer is "no", then I think we only need to support FunctionDecl and nothing else (not even ObjCMethodDecl, which is like a member function for ObjC). If the answer is "yes", then it's not clear to me whether lambdas should or should not be supported given that the attribute on the lambda expression is attached to the function call operator for the lambda declaration.

[gulfem]

clang/include/clang/Basic/Attr.td

1453

If the answer is "no", then I think we only need to support FunctionDecl and nothing else (not even ObjCMethodDecl, which is like a member function for ObjC). If the answer is "yes", then it's not clear to me whether lambdas should or should not be supported given that the attribute on the lambda expression is attached to the function call operator for the lambda declaration.

I see your point @aaron.ballman. I would say the second one is not really a library function.
@jdoerfert also suggested to allow leaf attribute only on declarations.
I can add FunctionDecl, so we only allow leaf attribute on function declarations, not on function definitions or member functions.
Does that sound good to both of you?

[aaron.ballman]

clang/include/clang/Basic/Attr.td

1453

I see your point @aaron.ballman. I would say the second one is not really a library function.

I feel like either they both are or they both aren't, but it's a question of how this attribute is intended to be used.

@jdoerfert also suggested to allow leaf attribute only on declarations.
I can add FunctionDecl, so we only allow leaf attribute on function declarations, not on function definitions or member functions.
Does that sound good to both of you?

I've come around to that approach, but FunctionDecl represents any declaration of a function, including a definition. So you'll probably want to add a new SubsetSubject in Attr.td to represent a function declaration that's not a definition (and we could potentially reuse that subject for a few other attributes that can't be written on a definition). You can use FunctionDecl::isThisDeclarationADefinition() to distinguish between declarations and definitions.

[gulfem]

clang/include/clang/Basic/Attr.td

1453

I feel like either they both are or they both aren't, but it's a question of how this attribute is intended to be used.

Sorry for being vague, and please let me try to clarify that.
What I meant was that existing leaf attribute cases are not like the cases that you provided.
It is used in library function declarations in Fuchsia and other existing use cases.
Are we ok banning this attribute in function definitions in clang even though this behavior is different than other compilers (GCC, ICC, etc.)?
If yes, I will incorporate the changes that you are suggesting.

[aaron.ballman]

clang/include/clang/Basic/Attr.td

1453

Sorry for being vague, and please let me try to clarify that.
What I meant was that existing leaf attribute cases are not like the cases that you provided.
It is used in library function declarations in Fuchsia and other existing use cases.

Okay, I think I'm on the same page as you now -- this attribute is most frequently written on free functions (ones that are not class members). However, I don't see a reason to disallow the attribute on a class member function though, or am I misunderstanding something? (GCC and ICC both seem to allow it on a class member function.)

Are we ok banning this attribute in function definitions in clang even though this behavior is different than other compilers (GCC, ICC, etc.)?

I don't think it's okay to *ban* use of this attribute on function definitions (e.g., we shouldn't reject the user's code) because that will make porting code more difficult, but I think diagnosing as a warning is reasonable..

This is what I think should happen: Let's drop the support for ObjCMethodDecl as that support can be added later if we find use cases that need it (this will make CodeGen easier in this patch).

Let's use a custom subject so that the attribute can only be written on a function declaration (which will automatically include member functions) but continue to not pass ErrorDiag in the SubjectList (so that we continue to warn rather than err if the subject is a function definition).

Let's not support blocks or lambdas unless a user comes up with use cases for it, but let's add tests to ensure that the behavior of the attribute on those is not harmful since the implicit methods generated for them may be a bit strange. For instance, the alias attribute cannot be written on a definition and yet: https://godbolt.org/z/vbbxKj To be clear -- I think the default behavior you get from the suggested SubjectList changes will be fine, but if it turns out that adding this attribute on a definition through a lambda causes harm (UB, crashes, etc) then we may have to put in extra effort to explicitly disallow it there.

And then add plenty of Sema tests for all of this so we're explicitly testing the behaviors we care about.

Does that sound reasonable to you?

[gulfem]

clang/include/clang/Basic/Attr.td

1453

Okay, I think I'm on the same page as you now -- this attribute is most frequently written on free functions (ones that are not class members). However, I don't see a reason to disallow the attribute on a class member function though, or am I misunderstanding something? (GCC and ICC both seem to allow it on a class member function.)

Your understanding is right. Technically, leaf attributes should be able to be used in methods as well.
However, I'm not aware of such existing cases.
As you suggested, I think we can extend leaf attribute support to methods and lambdas if we encounter such cases later.

Does that sound reasonable to you?

It sounds great! I agree with the plan, and I'll upload the changes in that direction.

[gulfem]

clang/include/clang/Basic/Attr.td

1453

@aaron.ballman I just added a simple rule for function declarations only.

def FunctionDeclOnly : SubsetSubject<Function,
                             [{!S->isThisDeclarationADefinition()}], 
                             "function declaration only">;

I used that one in the leaf attribute definition:

def Leaf : InheritableAttr {
  let Spellings = [GCC<"leaf">];
  let Subjects = SubjectList<[FunctionDeclOnly]>;
  let Documentation = [LeafDocs];
  let SimpleHandler = 1;
}

I thought that this will be straightforward, but after testing it on the following definition, surprisingly I did not get a warning.
I was expecting to get function declaration only warning.

__attribute__((leaf)) void f() 
{
}

After some debugging, I think this is what's happening:
When we parse the function attributes, body is not parsed yet.
As the following comment states in isThisDeclarationADefinition function, it returns false even for a definition.

/// Note: the function declaration does not become a definition until the
/// parser reaches the definition, if called before, this function will return
/// `false`.

Do you have any suggestions? Is there anything that I'm missing?

[gulfem]

clang/include/clang/Basic/Attr.td

1453

@aaron.ballman did you have a chance to take a look at my comment?

[aaron.ballman]

clang/include/clang/Basic/Attr.td

1453

Sorry about the delay in getting back to you on this (holiday schedule + C standards meetings got in the way of doing some reviews).

Ugh, that's a good point -- we've not seen the function body by the time we're processing attributes, so we don't know whether to diagnose or not. I can think of two ways forward:

0) Not diagnose when the attribute is written on a function definition.

1. Add some code to Sema::ActOnFinishFunctionBody to diagnose if the attribute appears on the declaration. However, I'm not certain if there's an easy way to distinguish between an attribute on the definition and an attribute inherited from the definition. e.g.,

[[gnu::leaf]] void func(void);
void func(void) { } // hasAttr<LeafAttr> on this will return true

I'm pretty sure that Attr::isInherited() reports whether the attribute should be inherited, not whether it actually has been inherited, so #1 could be tricky.

Personally, I'm fine with #0 if it turns out that #1 is painful. @jdoerfert, are you okay with that?

[gulfem]

clang/include/clang/Basic/Attr.td

1453

Personally, I'm fine with #0 if it turns out that #1 is painful. @jdoerfert, are you okay with that?

Would that be ok if we land this patch without diagnosing a warning, and work on a diagnosis patch later?

[aaron.ballman]

clang/include/clang/Basic/Attr.td

1453

I'd be fine with that!

[gulfem]

Only support leaf attribute in functions

[gulfem]

clang/include/clang/Basic/Attr.td

1453

I'd be fine with that!

@aaron.ballman would you please approve the patch so I can merge it to the master then?

[aaron.ballman]

Aside from a testing request, the frontend parts LGTM. I don't know enough about the LLVM side to feel comfortable signing off on it. @jdoerfert, would you mind approving that part?

clang/test/Sema/attr-leaf.c

5

Can you also add tests that show the attribute diagnoses being written on the wrong subject or when given arguments? Also, can you add a test showing the attribute on a function definition with a FIXME comment about wanting to diagnose that case?

[gulfem]

Added more Sema test cases

[gulfem]

clang/test/Sema/attr-leaf.c

5

I added three more test cases which also includes a test case with FXME comment for function definition diagnosis.

[aaron.ballman]

Thanks for the new tests!

clang/test/Sema/attr-leaf.c

13

Can you add the newline to the end of the file?

[gulfem]

Add a new line at the end of the test file

[jdoerfert]

Apologies for being silent for too long.

This is missing a lang ref entry for nocallback and the attributes.ll test is arguably broken (see below).
The "definition" in llvm/include/llvm/IR/Attributes.td (see below), does not match the the behavior of clang/test/CodeGen/attr-leaf.c.
As I mentioned before, this doesn't have a meaning on definitions and that needs to be captured in the semantics (and preferably the FE).

/// Function cannot enter into caller's translation unit.

llvm/test/Bitcode/attributes.ll

408

I kinda doubt this is going to work like it is supposed to.

[gulfem]

This is missing a lang ref entry for nocallback and the attributes.ll test is arguably broken (see below).

Could you please elaborate on missing lang ref entry? Where that should be added?

The "definition" in llvm/include/llvm/IR/Attributes.td (see below), does not match the the behavior of clang/test/CodeGen/attr-leaf.c.
As I mentioned before, this doesn't have a meaning on definitions and that needs to be captured in the semantics (and preferably the FE).

In that test case, leaf attribute is on the declaration.
What kind of a test case do you suggest to add?

[jdoerfert]

In D90275#2453641, @gulfem wrote:

This is missing a lang ref entry for nocallback and the attributes.ll test is arguably broken (see below).

Could you please elaborate on missing lang ref entry? Where that should be added?

Sure, my bad. Each enum attribute needs an entry in the language reference to define it's meaning, see also https://llvm.org/docs/LangRef.html#function-attributes
The content is in llvm/docs/LangRef.rst.

The "definition" in llvm/include/llvm/IR/Attributes.td (see below), does not match the the behavior of clang/test/CodeGen/attr-leaf.c.
As I mentioned before, this doesn't have a meaning on definitions and that needs to be captured in the semantics (and preferably the FE).

In that test case, leaf attribute is on the declaration.
What kind of a test case do you suggest to add?

We need to first define the meaning of nocallback in the IR, without that it is hard to do anything else.

---

The problem in llvm/test/Bitcode/attributes.ll is clear?

[xur]

This breaks llvm/test/Bitcode/attributes.ll.
The added test code was obviously wrong.

[gulfem]

The problem in llvm/test/Bitcode/attributes.ll is clear?

@xur Instead of define void @f70() nocallback, it should be define void @f69() nocallback, right?

[xur]

yes. Also #43 should be #42
I fixed it in
https://reviews.llvm.org/D92493
But that was just to make my test pass.

[gulfem]

I fixed it in
https://reviews.llvm.org/D92493
But that was just to make my test pass.

Ok, thank you!
I did not realize that typo after the merge, and sorry about that!
I fixed the typo that I introduced in https://reviews.llvm.org/D93420.

[gulfem]

Since this patch has been merged and the issue in the test has been resolved, I'm closing this review.

[nlopes]

Could you please add a description of this attribute to LangRef?
We need the semantics of this.

Thank you!

[gulfem]

In D90275#3671019, @nlopes wrote:

Could you please add a description of this attribute to LangRef?
We need the semantics of this.

Thank you!

Sure, I'll do that!

[nlopes]

In D90275#3671983, @gulfem wrote:

In D90275#3671019, @nlopes wrote:

Could you please add a description of this attribute to LangRef?
We need the semantics of this.

Thank you!

Sure, I'll do that!

ping?

[nlopes]

In D90275#3707793, @nlopes wrote:

In D90275#3671983, @gulfem wrote:

In D90275#3671019, @nlopes wrote:

Could you please add a description of this attribute to LangRef?
We need the semantics of this.

Thank you!

Sure, I'll do that!

ping?

Since the patch author has been ignoring my ask (and is active), and since documenting IR features is of paramount importance, I suggest we revert this patch. I think 3 weeks to document something is sufficient, and the author didn't provide an ETA.
I would like to ask everyone to reject all future patches that change the IR and don't include documentation, as experience says that we'll get miscompilations due to different people having different ideas of the semantics.

TL;DR: I'll revert this patch on Friday if no one chimes in. Thank you!

[gulfem]

In D90275#3712880, @nlopes wrote:

In D90275#3707793, @nlopes wrote:

In D90275#3671983, @gulfem wrote:

In D90275#3671019, @nlopes wrote:

Could you please add a description of this attribute to LangRef?
We need the semantics of this.

Thank you!

Sure, I'll do that!

ping?

Since the patch author has been ignoring my ask (and is active), and since documenting IR features is of paramount importance, I suggest we revert this patch. I think 3 weeks to document something is sufficient, and the author didn't provide an ETA.
I would like to ask everyone to reject all future patches that change the IR and don't include documentation, as experience says that we'll get miscompilations due to different people having different ideas of the semantics.

TL;DR: I'll revert this patch on Friday if no one chimes in. Thank you!

I'm very sorry and I was not ignoring your ask. I'll do it now.

[nlopes]

In D90275#3712969, @gulfem wrote:

In D90275#3712880, @nlopes wrote:

In D90275#3707793, @nlopes wrote:

In D90275#3671983, @gulfem wrote:

In D90275#3671019, @nlopes wrote:

Could you please add a description of this attribute to LangRef?
We need the semantics of this.

Thank you!

Sure, I'll do that!

ping?

Since the patch author has been ignoring my ask (and is active), and since documenting IR features is of paramount importance, I suggest we revert this patch. I think 3 weeks to document something is sufficient, and the author didn't provide an ETA.
I would like to ask everyone to reject all future patches that change the IR and don't include documentation, as experience says that we'll get miscompilations due to different people having different ideas of the semantics.

TL;DR: I'll revert this patch on Friday if no one chimes in. Thank you!

I'm very sorry and I was not ignoring your ask. I'll do it now.

Thanks!
I'm fine with a reasonable ETA, but we really need all IR features documented to avoid bugs.

[gulfem]

In D90275#3712991, @nlopes wrote:

In D90275#3712969, @gulfem wrote:

In D90275#3712880, @nlopes wrote:

In D90275#3707793, @nlopes wrote:

In D90275#3671983, @gulfem wrote:

In D90275#3671019, @nlopes wrote:

Could you please add a description of this attribute to LangRef?
We need the semantics of this.

Thank you!

Sure, I'll do that!

ping?

Since the patch author has been ignoring my ask (and is active), and since documenting IR features is of paramount importance, I suggest we revert this patch. I think 3 weeks to document something is sufficient, and the author didn't provide an ETA.
I would like to ask everyone to reject all future patches that change the IR and don't include documentation, as experience says that we'll get miscompilations due to different people having different ideas of the semantics.

TL;DR: I'll revert this patch on Friday if no one chimes in. Thank you!

I'm very sorry and I was not ignoring your ask. I'll do it now.

Thanks!
I'm fine with a reasonable ETA, but we really need all IR features documented to avoid bugs.

The urgency was not clear to me at the beginning, and I already added this to my list.
I did not have a chance to respond to your comment this week because of traveling.
But again, I did not mean to ignore this.

[gulfem]

Added the description in https://reviews.llvm.org/D131628

[xbolva00]

Was there any follow up? It seems that this hint (attribute) is currently not used by LLVM passes.

[jdoerfert]

In D90275#3715563, @xbolva00 wrote:

Was there any follow up? It seems that this hint (attribute) is currently not used by LLVM passes.

It is, also in our runtime, see https://reviews.llvm.org/D131628#3716252

clang/include/clang/Basic/Attr.td

 
 def LayoutVersion : InheritableAttr, TargetSpecificAttr<TargetMicrosoftCXXABI> {
   let Spellings = [Declspec<"layout_version">];
   let Args = [UnsignedArgument<"Version">];
   let Subjects = SubjectList<[CXXRecord]>;
   let Documentation = [LayoutVersionDocs];
 }
 
+def Leaf : InheritableAttr {
+  let Spellings = [GCC<"leaf">];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [LeafDocs];
+  let SimpleHandler = 1;
+}
+
 def LifetimeBound : DeclOrTypeAttr {
   let Spellings = [Clang<"lifetimebound", 0>];
   let Subjects = SubjectList<[ParmVar, ImplicitObjectParameter], ErrorDiag>;
   let Documentation = [LifetimeBoundDocs];
   let LangOpts = [CPlusPlus];
   let SimpleHandler = 1;
 }
 

clang/include/clang/Basic/AttrDocs.td

 applied to a definition. If ``allow_templates`` is given, template function
 definitions are considered as specializations of existing or assumed template
 declarations with the same name. The template parameters for the base functions
 are used to instantiate the specialization.
 
   }];
 }
 
+def LeafDocs : Documentation {
+  let Category = DocCatVariable;
+  let Content = [{
+
+The ``leaf`` attribute is used as a compiler hint to improve dataflow analysis
+in library functions. Functions marked with the ``leaf`` attribute are not allowed
+to jump back into the caller's translation unit, whether through invoking a
+callback function, an external function call, use of ``longjmp``, or other means.
+Therefore, they cannot use or modify any data that does not escape the caller function's
+compilation unit.
+
+For more information see
+`gcc documentation <https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html>`
+}];
+}
+
 def NoStackProtectorDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
 Clang supports the ``__attribute__((no_stack_protector))`` attribute which disables
 the stack protector on the specified function. This attribute is useful for
 selectively disabling the stack protector on some functions when building with
 ``-fstack-protector`` compiler option.
 

clang/lib/CodeGen/CGCall.cpp

if (TargetDecl->hasAttr<RestrictAttr>())
       RetAttrs.addAttribute(llvm::Attribute::NoAlias);
     if (TargetDecl->hasAttr<ReturnsNonNullAttr>() &&
         !CodeGenOpts.NullPointerIsValid)
       RetAttrs.addAttribute(llvm::Attribute::NonNull);
     if (TargetDecl->hasAttr<AnyX86NoCallerSavedRegistersAttr>())
       FuncAttrs.addAttribute("no_caller_saved_registers");
     if (TargetDecl->hasAttr<AnyX86NoCfCheckAttr>())
       FuncAttrs.addAttribute(llvm::Attribute::NoCfCheck);
+    if (TargetDecl->hasAttr<LeafAttr>())
+      FuncAttrs.addAttribute(llvm::Attribute::NoCallback);
 
     HasOptnone = TargetDecl->hasAttr<OptimizeNoneAttr>();
     if (auto *AllocSize = TargetDecl->getAttr<AllocSizeAttr>()) {
       Optional<unsigned> NumElemsParam;
       if (AllocSize->getNumElemsParam().isValid())
         NumElemsParam = AllocSize->getNumElemsParam().getLLVMIndex();
       FuncAttrs.addAllocSizeAttr(AllocSize->getElemSizeParam().getLLVMIndex(),
                                  NumElemsParam);

clang/lib/Sema/SemaDeclAttr.cpp

case ParsedAttr::AT_Naked:
     handleNakedAttr(S, D, AL);
     break;
   case ParsedAttr::AT_NoReturn:
     handleNoReturnAttr(S, D, AL);
     break;
   case ParsedAttr::AT_AnyX86NoCfCheck:
     handleNoCfCheckAttr(S, D, AL);
     break;
+  case ParsedAttr::AT_Leaf:
+    handleSimpleAttribute<LeafAttr>(S, D, AL);
+    break;
   case ParsedAttr::AT_NoThrow:
     if (!AL.isUsedAsTypeAttr())
       handleSimpleAttribute<NoThrowAttr>(S, D, AL);
     break;
   case ParsedAttr::AT_CUDAShared:
     handleSharedAttr(S, D, AL);
     break;
   case ParsedAttr::AT_VecReturn:

clang/test/CodeGen/attr-leaf.c

+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -Oz -o - %s | FileCheck %s
+
+// CHECK: define void @f() local_unnamed_addr [[ATTRS:#[0-9]+]] {
+void f() __attribute__((leaf));
+
+void f()
+{
+}
+
+// CHECK: attributes [[ATTRS]] = { {{.*}}nocallback{{.*}} }

clang/test/Misc/pragma-attribute-supported-attributes-list.test

 // CHECK-NEXT: Flatten (SubjectMatchRule_function)
 // CHECK-NEXT: GNUInline (SubjectMatchRule_function)
 // CHECK-NEXT: Hot (SubjectMatchRule_function)
 // CHECK-NEXT: IBAction (SubjectMatchRule_objc_method_is_instance)
 // CHECK-NEXT: IFunc (SubjectMatchRule_function)
 // CHECK-NEXT: InitPriority (SubjectMatchRule_variable)
 // CHECK-NEXT: InternalLinkage (SubjectMatchRule_variable, SubjectMatchRule_function, SubjectMatchRule_record)
 // CHECK-NEXT: LTOVisibilityPublic (SubjectMatchRule_record)
+// CHECK-NEXT: Leaf (SubjectMatchRule_function)
 // CHECK-NEXT: LoaderUninitialized (SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: Lockable (SubjectMatchRule_record)
 // CHECK-NEXT: MIGServerRoutine (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_block)
 // CHECK-NEXT: MSStruct (SubjectMatchRule_record)
 // CHECK-NEXT: MicroMips (SubjectMatchRule_function)
 // CHECK-NEXT: MinSize (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: MinVectorWidth (SubjectMatchRule_function)
 // CHECK-NEXT: Mips16 (SubjectMatchRule_function)

clang/test/Sema/attr-leaf.c

+// RUN: %clang_cc1 -verify -fsyntax-only %s
+
+void f1() __attribute__((leaf));
+
+void f2() __attribute__((leaf("abc"))); // expected-error {{'leaf' attribute takes no argument}}
+
+int var __attribute__ ((leaf())); // expected-warning {{'leaf' attribute only applies to functions}}
+
+// FIXME: Might diagnose a warning if leaf attribute is used in function definition
+// The leaf attribute has no effect on functions defined within the current compilation unit
+__attribute__((leaf)) void f3() {
+}

llvm/include/llvm/Bitcode/LLVMBitCodes.h

enum AttributeKindCodes {
   ATTR_KIND_NOSYNC = 63,
   ATTR_KIND_SANITIZE_MEMTAG = 64,
   ATTR_KIND_PREALLOCATED = 65,
   ATTR_KIND_NO_MERGE = 66,
   ATTR_KIND_NULL_POINTER_IS_VALID = 67,
   ATTR_KIND_NOUNDEF = 68,
   ATTR_KIND_BYREF = 69,
   ATTR_KIND_MUSTPROGRESS = 70,
+  ATTR_KIND_NO_CALLBACK = 71,
 };
 
 enum ComdatSelectionKindCodes {
   COMDAT_SELECTION_KIND_ANY = 1,
   COMDAT_SELECTION_KIND_EXACT_MATCH = 2,
   COMDAT_SELECTION_KIND_LARGEST = 3,
   COMDAT_SELECTION_KIND_NO_DUPLICATES = 4,
   COMDAT_SELECTION_KIND_SAME_SIZE = 5,

llvm/include/llvm/IR/Attributes.td

 def Nest : EnumAttr<"nest">;
 
 /// Considered to not alias after call.
 def NoAlias : EnumAttr<"noalias">;
 
 /// Callee isn't recognized as a builtin.
 def NoBuiltin : EnumAttr<"nobuiltin">;
 
+/// Function cannot enter into caller's translation unit.
+def NoCallback : EnumAttr<"nocallback">;
+
 /// Function creates no aliases of pointer.
 def NoCapture : EnumAttr<"nocapture">;
 
 /// Call cannot be duplicated.
 def NoDuplicate : EnumAttr<"noduplicate">;
 
 /// Function does not deallocate memory.
 def NoFree : EnumAttr<"nofree">;

llvm/lib/AsmParser/LLLexer.cpp

#define KEYWORD(STR)                                                           \
   KEYWORD(inlinehint);
   KEYWORD(inreg);
   KEYWORD(jumptable);
   KEYWORD(minsize);
   KEYWORD(naked);
   KEYWORD(nest);
   KEYWORD(noalias);
   KEYWORD(nobuiltin);
+  KEYWORD(nocallback);
   KEYWORD(nocapture);
   KEYWORD(noduplicate);
   KEYWORD(nofree);
   KEYWORD(noimplicitfloat);
   KEYWORD(noinline);
   KEYWORD(norecurse);
   KEYWORD(nonlazybind);
   KEYWORD(nomerge);

llvm/lib/AsmParser/LLParser.cpp

while (true) {
     case lltok::kw_inlinehint: B.addAttribute(Attribute::InlineHint); break;
     case lltok::kw_jumptable: B.addAttribute(Attribute::JumpTable); break;
     case lltok::kw_minsize: B.addAttribute(Attribute::MinSize); break;
     case lltok::kw_mustprogress:
       B.addAttribute(Attribute::MustProgress);
       break;
     case lltok::kw_naked: B.addAttribute(Attribute::Naked); break;
     case lltok::kw_nobuiltin: B.addAttribute(Attribute::NoBuiltin); break;
+    case lltok::kw_nocallback:
+      B.addAttribute(Attribute::NoCallback);
+      break;
     case lltok::kw_noduplicate: B.addAttribute(Attribute::NoDuplicate); break;
     case lltok::kw_nofree: B.addAttribute(Attribute::NoFree); break;
     case lltok::kw_noimplicitfloat:
       B.addAttribute(Attribute::NoImplicitFloat); break;
     case lltok::kw_noinline: B.addAttribute(Attribute::NoInline); break;
     case lltok::kw_nonlazybind: B.addAttribute(Attribute::NonLazyBind); break;
     case lltok::kw_nomerge: B.addAttribute(Attribute::NoMerge); break;
     case lltok::kw_noredzone: B.addAttribute(Attribute::NoRedZone); break;

llvm/lib/AsmParser/LLToken.h

enum Kind {
   kw_inreg,
   kw_jumptable,
   kw_minsize,
   kw_naked,
   kw_nest,
   kw_noalias,
   kw_noundef,
   kw_nobuiltin,
+  kw_nocallback,
   kw_nocapture,
   kw_noduplicate,
   kw_nofree,
   kw_noimplicitfloat,
   kw_noinline,
   kw_norecurse,
   kw_nonlazybind,
   kw_nomerge,

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

static Attribute::AttrKind getAttrFromCode(uint64_t Code) {
   case bitc::ATTR_KIND_NAKED:
     return Attribute::Naked;
   case bitc::ATTR_KIND_NEST:
     return Attribute::Nest;
   case bitc::ATTR_KIND_NO_ALIAS:
     return Attribute::NoAlias;
   case bitc::ATTR_KIND_NO_BUILTIN:
     return Attribute::NoBuiltin;
+  case bitc::ATTR_KIND_NO_CALLBACK:
+    return Attribute::NoCallback;
   case bitc::ATTR_KIND_NO_CAPTURE:
     return Attribute::NoCapture;
   case bitc::ATTR_KIND_NO_DUPLICATE:
     return Attribute::NoDuplicate;
   case bitc::ATTR_KIND_NOFREE:
     return Attribute::NoFree;
   case bitc::ATTR_KIND_NO_IMPLICIT_FLOAT:
     return Attribute::NoImplicitFloat;

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

static uint64_t getAttrKindEncoding(Attribute::AttrKind Kind) {
   case Attribute::Naked:
     return bitc::ATTR_KIND_NAKED;
   case Attribute::Nest:
     return bitc::ATTR_KIND_NEST;
   case Attribute::NoAlias:
     return bitc::ATTR_KIND_NO_ALIAS;
   case Attribute::NoBuiltin:
     return bitc::ATTR_KIND_NO_BUILTIN;
+  case Attribute::NoCallback:
+    return bitc::ATTR_KIND_NO_CALLBACK;
   case Attribute::NoCapture:
     return bitc::ATTR_KIND_NO_CAPTURE;
   case Attribute::NoDuplicate:
     return bitc::ATTR_KIND_NO_DUPLICATE;
   case Attribute::NoFree:
     return bitc::ATTR_KIND_NOFREE;
   case Attribute::NoImplicitFloat:
     return bitc::ATTR_KIND_NO_IMPLICIT_FLOAT;

llvm/lib/IR/Attributes.cpp

std::string Attribute::getAsString(bool InAttrGrp) const {
   if (hasAttribute(Attribute::Naked))
     return "naked";
   if (hasAttribute(Attribute::Nest))
     return "nest";
   if (hasAttribute(Attribute::NoAlias))
     return "noalias";
   if (hasAttribute(Attribute::NoBuiltin))
     return "nobuiltin";
+  if (hasAttribute(Attribute::NoCallback))
+    return "nocallback";
   if (hasAttribute(Attribute::NoCapture))
     return "nocapture";
   if (hasAttribute(Attribute::NoDuplicate))
     return "noduplicate";
   if (hasAttribute(Attribute::NoFree))
     return "nofree";
   if (hasAttribute(Attribute::NoImplicitFloat))
     return "noimplicitfloat";

llvm/lib/IR/Verifier.cpp

 
 /// Return true if this attribute kind only applies to functions.
 static bool isFuncOnlyAttr(Attribute::AttrKind Kind) {
   switch (Kind) {
   case Attribute::NoMerge:
   case Attribute::NoReturn:
   case Attribute::NoSync:
   case Attribute::WillReturn:
+  case Attribute::NoCallback:
   case Attribute::NoCfCheck:
   case Attribute::NoUnwind:
   case Attribute::NoInline:
   case Attribute::AlwaysInline:
   case Attribute::OptimizeForSize:
   case Attribute::StackProtect:
   case Attribute::StackProtectReq:
   case Attribute::StackProtectStrong:

llvm/lib/Transforms/Utils/CodeExtractor.cpp

if (Attr.isStringAttribute()) {
       case Attribute::TombstoneKey:
         continue;
       // Those attributes should be safe to propagate to the extracted function.
       case Attribute::AlwaysInline:
       case Attribute::Cold:
       case Attribute::NoRecurse:
       case Attribute::InlineHint:
       case Attribute::MinSize:
+      case Attribute::NoCallback:
       case Attribute::NoDuplicate:
       case Attribute::NoFree:
       case Attribute::NoImplicitFloat:
       case Attribute::NoInline:
       case Attribute::NonLazyBind:
       case Attribute::NoRedZone:
       case Attribute::NoUnwind:
       case Attribute::NullPointerIsValid:

llvm/test/Bitcode/attributes.ll

 }
 
 ; CHECK: define void @f68() #41
 define void @f68() mustprogress
 {
   ret void
 }
 
+; CHECK; define void @f69() #43
+define void @f70() nocallback
+{
+  ret void
+}
+
 ; CHECK: attributes #0 = { noreturn }
 ; CHECK: attributes #1 = { nounwind }
 ; CHECK: attributes #2 = { readnone }
 ; CHECK: attributes #3 = { readonly }
 ; CHECK: attributes #4 = { noinline }
 ; CHECK: attributes #5 = { alwaysinline }
 ; CHECK: attributes #6 = { optsize }
 ; CHECK: attributes #7 = { ssp }

 ; CHECK: attributes #34 = { sanitize_hwaddress }
 ; CHECK: attributes #35 = { shadowcallstack }
 ; CHECK: attributes #36 = { willreturn }
 ; CHECK: attributes #37 = { nofree }
 ; CHECK: attributes #38 = { nosync }
 ; CHECK: attributes #39 = { sanitize_memtag }
 ; CHECK: attributes #40 = { null_pointer_is_valid }
 ; CHECK: attributes #41 = { mustprogress }
+; CHECK: attributes #42 = { nocallback }
 ; CHECK: attributes #[[NOBUILTIN]] = { nobuiltin }