Implement __attribute__((gc_leaf_function)).

[mjacob]

Summary

This attribute can be applied to functions, and results in declarations and
calls of the function have the "gc-leaf-function" LLVM IR attribute.

[reames]

Neither Sanjoy or I are qualified to review a clang patch. You'll need to find clang reviewers.

Also, before this gets exposed through Clang, we really should formalize/document the attribute. In practice, it implies the lack of a safepoint poll site inside the called function. Annoyingly, it's not an inferable property since we don't represent the possible insertion of a poll in the IR.

Hm. This makes me wonder... We've moved to a model of inserting safepoints (specifically for deopt info) early, and then rewriting late in our tree. We're not even using the PlaceSafepoints pass any more. It's been left mostly for other users. Would it maybe make sense to fully retire PlaceSafepoints and migrate over to a scheme where safepoints sites are explicit from the beginning? This would allow us to infer "gc-leaf" from FunctionAttrs...

(This high level discussion should probably move to llvm-dev. I can start it if you'd like, otherwise post something and I'll reply.)

[majnemer]

This change looks fine modulo the documentation issue.

include/clang/Basic/Attr.td

2177

@aaron.ballman prefers that all attributes we add have some documentation. This documentation would live in AttrDocs.td.

[mjacob]

In D15998#324354, @reames wrote:

Also, before this gets exposed through Clang, we really should formalize/document the attribute. In practice, it implies the lack of a safepoint poll site inside the called function. Annoyingly, it's not an inferable property since we don't represent the possible insertion of a poll in the IR.

That wouldn't work for my language since it doesn't use polls at all. Instead, a practical definition would be: "a statepoint will not be inserted if the call site or callee has this attribute". My language backend currently relies on this, e.g. if calling printf (currently statepoints don't support vararg callees).

Hm. This makes me wonder... We've moved to a model of inserting safepoints (specifically for deopt info) early, and then rewriting late in our tree. We're not even using the PlaceSafepoints pass any more. It's been left mostly for other users. Would it maybe make sense to fully retire PlaceSafepoints and migrate over to a scheme where safepoints sites are explicit from the beginning? This would allow us to infer "gc-leaf" from FunctionAttrs...

With explicit safepoints and the above-mentioned definition the gc-leaf-funtion attribute wouldn't be necessary at all, because then a function is a GC leaf if and only if no safepoint should be inserted when calling this function.

(This high level discussion should probably move to llvm-dev. I can start it if you'd like, otherwise post something and I'll reply.)

I think it makes more sense if you start the discussion because I'm not familiar with your use cases (I only use a subset of safepoint functionality, e.g. no safepoint polls).

[aaron.ballman]

Can you point me to some documentation on what the semantics of this attribute are? For instance, how does it play with other attributes (like naked or dllexport), is there a reason it shouldn't apply to Objective-C methods, etc?

include/clang/Basic/Attr.td

2175

Any particular reason for this to not have a C++11 spelling under the clang namespace, in addition to the GNU-style spelling?

[mjacob]

In D15998#324757, @aaron.ballman wrote:

Can you point me to some documentation on what the semantics of this attribute are? For instance, how does it play with other attributes (like naked or dllexport), is there a reason it shouldn't apply to Objective-C methods, etc?

As it was noted earlier in this review, this attribute (as its underlying LLVM attribute) is underspecified. We should discuss the semantics (and whether we want to keep it in the first place) of the LLVM attribute on the mailing list. I'm not sure how we should proceed with this patch in the meantime, probably one of:

1. Mark this (Clang) attribute as tentative, and remove it in case we remove the LLVM attribute.
2. Close this revision and create a new patch depending on the outcome of the discussion.
3. Postpone (but not close) this revision.

include/clang/Basic/Attr.td

2175

I'll add this in case we decide the attribute is the right approach.

[aaron.ballman]

In D15998#324821, @mjacob wrote:

In D15998#324757, @aaron.ballman wrote:

Can you point me to some documentation on what the semantics of this attribute are? For instance, how does it play with other attributes (like naked or dllexport), is there a reason it shouldn't apply to Objective-C methods, etc?

As it was noted earlier in this review, this attribute (as its underlying LLVM attribute) is underspecified. We should discuss the semantics (and whether we want to keep it in the first place) of the LLVM attribute on the mailing list. I'm not sure how we should proceed with this patch in the meantime, probably one of:

1. Mark this (Clang) attribute as tentative, and remove it in case we remove the LLVM attribute.
2. Close this revision and create a new patch depending on the outcome of the discussion.
3. Postpone (but not close) this revision.

Given how relatively easy this patch is at this stage, I would recommend #2 if the LLVM side of the discussion is going to take more than a month, and #3 otherwise.

[sanjoy]

Resigning for now to make my "Revisions Waiting on You" queue less noisy. Please don't hesitate to add me back if this or a variant of this change becomes active.

[mjacob]

I experimented with another approach in the meantime.

include/clang/Basic/Attr.td

def OMPThreadPrivateDecl : InheritableAttr {
   let Documentation = [Undocumented];
 }
 
 def InternalLinkage : InheritableAttr {
   let Spellings = [GNU<"internal_linkage">, CXX11<"clang", "internal_linkage">];
   let Subjects = SubjectList<[Var, Function, CXXRecord]>;
   let Documentation = [InternalLinkageDocs];
 }
+
+def GcLeafFunction : InheritableAttr {
+  let Spellings = [GNU<"gc_leaf_function">];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [Undocumented];
+}

lib/CodeGen/CGCall.cpp

if (TargetDecl) {
     } else if (TargetDecl->hasAttr<NoAliasAttr>()) {
       FuncAttrs.addAttribute(llvm::Attribute::ArgMemOnly);
       FuncAttrs.addAttribute(llvm::Attribute::NoUnwind);
     }
     if (TargetDecl->hasAttr<RestrictAttr>())
       RetAttrs.addAttribute(llvm::Attribute::NoAlias);
     if (TargetDecl->hasAttr<ReturnsNonNullAttr>())
       RetAttrs.addAttribute(llvm::Attribute::NonNull);
+    if (TargetDecl->hasAttr<GcLeafFunctionAttr>())
+      FuncAttrs.addAttribute("gc-leaf-function");
 
     HasOptnone = TargetDecl->hasAttr<OptimizeNoneAttr>();
   }
 
   // OptimizeNoneAttr takes precedence over -Os or -Oz. No warning needed.
   if (!HasOptnone) {
     if (CodeGenOpts.OptimizeSize)
       FuncAttrs.addAttribute(llvm::Attribute::OptimizeForSize);

lib/Sema/SemaDeclAttr.cpp

case AttributeList::AT_OpenCLKernel:
     handleSimpleAttribute<OpenCLKernelAttr>(S, D, Attr);
     break;
   case AttributeList::AT_OpenCLImageAccess:
     handleSimpleAttribute<OpenCLImageAccessAttr>(S, D, Attr);
     break;
   case AttributeList::AT_InternalLinkage:
     handleInternalLinkageAttr(S, D, Attr);
     break;
+  case AttributeList::AT_GcLeafFunction:
+    handleSimpleAttribute<GcLeafFunctionAttr>(S, D, Attr);
+    break;
 
   // Microsoft attributes:
   case AttributeList::AT_MSNoVTable:
     handleSimpleAttribute<MSNoVTableAttr>(S, D, Attr);
     break;
   case AttributeList::AT_MSStruct:
     handleSimpleAttribute<MSStructAttr>(S, D, Attr);
     break;

test/CodeGen/attr-gc-leaf-function.c

+// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s
+
+void foo(void) __attribute__((gc_leaf_function));
+
+void test(void) {
+    // CHECK: call void @foo() [[ATTRS_CALL:#[0-9]+]]
+    foo();
+}
+
+// CHECK: declare void @foo() [[ATTRS_DECL:#[0-9]+]]
+// CHECK: attributes [[ATTRS_DECL]] = { {{.*}}"gc-leaf-function"{{.*}} }
+// CHECK: attributes [[ATTRS_CALL]] = { {{.*}}"gc-leaf-function"{{.*}} }