[CGCall] Add NoInline attr if presented for the target

[djtodoro]

Summary

The CodeGen is missing to add the NoInline attribute to the target of a call, even the declaration has the attribute presented. This fixes it.

[rjmccall]

Why do you want to add noinline to a function declaration that lacks a definition?

[djtodoro]

In D108618#2964265, @rjmccall wrote:

Why do you want to add noinline to a function declaration that lacks a definition?

If this won't be used at all I guess the compiler should throw a warning at least.
One example comes to my mind, and it includes LTO (and cross-compile unit inlining).

$ cat test-1.c
extern int __attribute__((noinline)) bar(void);

int foo() {
int v1 = bar();
if (v1 > 100)
  return 0;
v1--;
return v1;
}

$ cat $ cat test-2.c
int __attribute__((always_inline)) bar(void);

int foo2() {
int v2 = bar();   
if (v2 > 1000)
  return 0;
v2++;
return v2;
}

$ cat test.c
#include <stdio.h>

extern int foo();
extern int foo2();

int bar() {
int x;
scanf("%d", &x);
++x;
return x;
}


int main()
{
int i = foo();
int j = foo2();
return i - j;
}

$ clang test.c -g -S -flto -emit-llvm
$ clang test-1.c -g -S -flto -emit-llvm
$ clang test-2.c -g -S -flto -emit-llvm

$ llvm-link -S -o linked.ll test-1.ll test-2.ll test.ll
$ opt -S -O1 linked.ll -o optimized.ll

Having these simple compile units, I can expect that compiler is able to inline the bar() into foo2() but not into foo().

[rjmccall]

Does LLVM model noinline as a call-site attribute in the way that would be necessary to get that effect? Also, are you actually having a problem here, or is this just something you noticed in the code?

I'm not sure we can warn about putting the attribute on a declaration; it's presumably still picked up by later definitions. There's probably a warning if they conflict, as they would if the first declaration was in a header included in both places, which is best practice.

[djtodoro]

In D108618#2968626, @rjmccall wrote:

Does LLVM model noinline as a call-site attribute in the way that would be necessary to get that effect? Also, are you actually having a problem here, or is this just something you noticed in the code?

It is being ignored completely, anyway :/ I am not having a problem, actually. It has been noticed accidentally when looking at some other attribute, but then I've started playing with some Debug Info cases (since that is the area I mostly work on) by dancing around with some cross CU referencing when using LTO -- all in all, this isn't necessary.

I'm not sure we can warn about putting the attribute on a declaration; it's presumably still picked up by later definitions. There's probably a warning if they conflict, as they would if the first declaration was in a header included in both places, which is best practice.

Oh yes... It is very hard to warn during compilation since we don't have the definition -- even though we have conflicts in test-1.c and test-2.c.

@rjmccall Thanks for your comments anyway :)

[rjmccall]

Alright, no worries. It was reasonable to ask if this was something we should fix.

clang/lib/CodeGen/CGCall.cpp

if (TargetDecl->hasAttr<ColdAttr>())
       FuncAttrs.addAttribute(llvm::Attribute::Cold);
     if (TargetDecl->hasAttr<HotAttr>())
       FuncAttrs.addAttribute(llvm::Attribute::Hot);
     if (TargetDecl->hasAttr<NoDuplicateAttr>())
       FuncAttrs.addAttribute(llvm::Attribute::NoDuplicate);
     if (TargetDecl->hasAttr<ConvergentAttr>())
       FuncAttrs.addAttribute(llvm::Attribute::Convergent);
 
+    if (!AttrOnCallSite && TargetDecl->hasAttr<NoInlineAttr>())
+      FuncAttrs.addAttribute(llvm::Attribute::NoInline);
+
     if (const FunctionDecl *Fn = dyn_cast<FunctionDecl>(TargetDecl)) {
       AddAttributesFromFunctionProtoType(
           getContext(), FuncAttrs, Fn->getType()->getAs<FunctionProtoType>());
       if (AttrOnCallSite && Fn->isReplaceableGlobalAllocationFunction()) {
         // A sane operator new returns a non-aliasing pointer.
         auto Kind = Fn->getDeclName().getCXXOverloadedOperator();
         if (getCodeGenOpts().AssumeSaneOperatorNew &&
             (Kind == OO_New || Kind == OO_Array_New))

clang/lib/CodeGen/CGClass.cpp

case Dtor_Base:
     else if (Body)
       EmitStmt(Body);
     else {
       assert(Dtor->isImplicit() && "bodyless dtor not implicit");
       // nothing to do besides what's in the epilogue
     }
     // -fapple-kext must inline any call to this dtor into
     // the caller's body.
-    if (getLangOpts().AppleKext)
+    if (getLangOpts().AppleKext) {
+      if (CurFn->hasFnAttribute(llvm::Attribute::NoInline))
+        CurFn->removeFnAttr(llvm::Attribute::NoInline);
       CurFn->addFnAttr(llvm::Attribute::AlwaysInline);
+    }
 
     break;
   }
 
   // Jump out through the epilogue cleanups.
   DtorEpilogue.ForceCleanup();
 
   // Exit the try if applicable.

clang/test/CodeGen/noinline-attr-on-target.c

+// Make sure the `noinline` attribute has been attached to the `bar()`.
+
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: define dso_local void @foo() #[[#ATTR0:]] {
+// CHECK: declare void @bar() #[[#ATTR1:]]
+// CHECK: attributes #[[#ATTR0]] = { noinline
+// CHECK: attributes #[[#ATTR1]] = { noinline
+
+extern void __attribute__((noinline)) bar(void);
+void foo() {
+  bar();
+}