[clang] Allow 'nomerge' attribute for function pointers

[eddyz87]

Summary

Allow specifying 'nomerge' attribute for function pointers,
e.g. like in the following C code:

extern void (*foo)(void) __attribute__((nomerge));
void bar(long i) {
  if (i)
    foo();
  else
    foo();
}

With the goal to attach 'nomerge' to both calls done through 'foo':

@foo = external local_unnamed_addr global ptr, align 8
define dso_local void @bar(i64 noundef %i) local_unnamed_addr #0 {
  ; ...
  %0 = load ptr, ptr @foo, align 8, !tbaa !5
  ; ...
if.then:
  tail call void %0() #1
  br label %if.end
if.else:
  tail call void %0() #1
  br label %if.end
if.end:
  ret void
}
; ...
attributes #1 = { nomerge ... }

Report a warning in case if 'nomerge' is specified for a variable that
is not a function pointer, e.g.:

t.c:2:22: warning: 'nomerge' attribute is ignored because 'j' is not a function pointer [-Wignored-attributes]
    2 | int j __attribute__((nomerge));
      |                      ^

The intended use-case is for BPF backend.

BPF provides a sort of "standard library" functions that are called
helpers. BPF also verifies usage of these helpers before program
execution. Because of limitations of verification / runtime model it
is important to keep calls to some of such helpers from merging.

An example could be found by the link [1], there input C code:

if (data_end - data > 1024) {
    bpf_for_each_map_elem(&map1, cb, &cb_data, 0);
} else {
    bpf_for_each_map_elem(&map2, cb, &cb_data, 0);
}

Is converted to bytecode equivalent to:

if (data_end - data > 1024)
  tmp = &map1;
else
  tmp = &map2;
bpf_for_each_map_elem(tmp, cb, &cb_data, 0);

However, BPF verification/runtime requires to use the same map address
for each particular bpf_for_each_map_elem() call.

The 'nomerge' attribute is a perfect match for this situation, but
unfortunately BPF helpers are declared as pointers to functions:

static long (*bpf_for_each_map_elem)(void *map, ...) = (void *) 164;

Hence, this commit, allowing to use 'nomerge' for function pointers.

[1] https://lore.kernel.org/bpf/[e-mail]/

[eddyz87]

Hi @aaron.ballman, @rnk,

I see that you were involved in the discussion when nomerge attribute was added in D79121.
Could you please take a look at this proposed change?
It would be useful for BPF back-end and I tried to explain the use-case in the revision description.

[eddyz87]

Hi @jemarch, @dfaust,

You might be interested in this discussion.

[rnk]

The purpose of the attribute is really limited to preserving source location information on instructions, and this isn't really a supported usage. The BPF backend and verifier needs to learn to tolerate valid LLVM transforms if it wants to be a real LLVM backend. Of course, you can do what you like.

Considered in the context of the original use case, I think it's reasonable to allow the attribute on function pointers for the same reasons we allow it on function declarations. It makes it easy to work the attribute onto the direct call sites of the function without modifying tons of source code. However, I'd like to see clearer documentation on the limitations.

clang/include/clang/Basic/AttrDocs.td

550–577

This statement of the attribute having "no effect on indirect calls" is slightly confusing now that we talk about function pointers immediately afterward. Can you please rework this a bit, and clarify that when applied to function pointers, the attribute only takes effect when the call target is directly the variable which carries the attribute? For example, this has no effect:

void (*fp)() __attribute__((nomerge));
void callit() {
  auto tmp = fp;
  tmp();
  (*fp)(); // I think TargetDecl will be null in the code, tell me if I'm wrong
}

[eddyz87]

In D152986#4425736, @rnk wrote:

Thank you for the review!

The purpose of the attribute is really limited to preserving source location information on instructions, and this isn't really a supported usage.

But it would prevent merging, right? I understand that using this attribute might block some legitimate optimizations.
Or do you mean that it is a "best effort" thing and not all transformations might honor it?

The BPF backend and verifier needs to learn to tolerate valid LLVM transforms if it wants to be a real LLVM backend. Of course, you can do what you like.

Well, I agree with that. The issue here is with the map API design: it is polymorphic, but it is expected that at load time verifier can replace all polymorphic calls with static calls.
People know it and write code in a way that allows verifier to infer which static functions to call. So I have limited options here:

• either adjust IR at early stages of pipeline so that specific calls are not altered;
• or do nothing and recommend users to use [[clang::nomerge]] on statement level or insert something like asm volatile ("" :::"memory") here and there.

But, yeah, I understand that is is not a C language semantics and headache is mine.

Considered in the context of the original use case, I think it's reasonable to allow the attribute on function pointers for the same reasons we allow it on function declarations. It makes it easy to work the attribute onto the direct call sites of the function without modifying tons of source code. However, I'd like to see clearer documentation on the limitations.

Please see my "inline" response.

clang/include/clang/Basic/AttrDocs.td

550–577

I can make it more elaborate, would the text as below be fine?
Regarding TargetDecl value it is not null both times:

• (VarDecl 'tmp' (ImplicitCastExpr (DeclRefExpr (Var fp))))
• (VarDecl 'fp')

---

`nomerge` attribute can also be used as function attribute to prevent all
calls to the specified function from merging. It has no effect on indirect
calls to such functions. For example:

.. code-block:: c++

[[clang::nomerge]] void foo(int) {}

void bar(int x) {
  auto *ptr = foo;
  if (x) foo(1); else foo(2); // will not be merged
  if (x) ptr(1); else ptr(2); // indirect call, can be merged
}

`nomerge` attribute can also be used for pointers to functions to
prevent calls through such pointer from merging. In such case the
effect applies only to a specific function pointer. For example:

.. code-block:: c++

[[clang::nomerge]] void (*foo)(int);

void bar(int x) {
  auto *ptr = foo;
  if (x) foo(1); else foo(2); // will not be merged
  if (x) ptr(1); else ptr(2); // 'ptr' has no 'nomerge' attribute,
                              // can be merged
}

[rnk]

In D152986#4426256, @eddyz87 wrote:

In D152986#4425736, @rnk wrote:

The purpose of the attribute is really limited to preserving source location information on instructions, and this isn't really a supported usage.

But it would prevent merging, right? I understand that using this attribute might block some legitimate optimizations.
Or do you mean that it is a "best effort" thing and not all transformations might honor it?

Essentially, yes.

Anyway, I think this looks good, please apply the doc update.

clang/include/clang/Basic/AttrDocs.td

550–577

Looks good to me, thanks!

[eddyz87]

Rebase, nomerge documentation updated.

[eddyz87]

Rebase

clang/include/clang/Basic/Attr.td

def Unlikely : StmtAttr {
   let Spellings = [CXX11<"", "unlikely", 201803>, C2x<"clang", "unlikely">];
   let Documentation = [LikelihoodDocs];
 }
 def : MutualExclusions<[Likely, Unlikely]>;
 
 def NoMerge : DeclOrStmtAttr {
   let Spellings = [Clang<"nomerge">];
   let Documentation = [NoMergeDocs];
-  let Subjects = SubjectList<[Function, Stmt], ErrorDiag,
-                             "functions and statements">;
-  let SimpleHandler = 1;
+  let Subjects = SubjectList<[Function, Stmt, Var], ErrorDiag,
+                             "functions, statements and variables">;
 }
 
 def MustTail : StmtAttr {
   let Spellings = [Clang<"musttail">];
   let Documentation = [MustTailDocs];
   let Subjects = SubjectList<[ReturnStmt], ErrorDiag, "return statements">;
 }
 

clang/include/clang/Basic/AttrDocs.td

 expressions inside the statement will not be merged during optimization. This
 attribute can be used to prevent the optimizer from obscuring the source
 location of certain calls. For example, it will prevent tail merging otherwise
 identical code sequences that raise an exception or terminate the program. Tail
 merging normally reduces the precision of source location information, making
 stack traces less useful for debugging. This attribute gives the user control
 over the tradeoff between code size and debug information precision.
 
 ``nomerge`` attribute can also be used as function attribute to prevent all
 calls to the specified function from merging. It has no effect on indirect
-calls.
+calls to such functions. For example:
+
+.. code-block:: c++
+
+  [[clang::nomerge]] void foo(int) {}
+
+  void bar(int x) {
+    auto *ptr = foo;
+    if (x) foo(1); else foo(2); // will not be merged
+    if (x) ptr(1); else ptr(2); // indirect call, can be merged
+  }
+
+``nomerge`` attribute can also be used for pointers to functions to
+prevent calls through such pointer from merging. In such case the
+effect applies only to a specific function pointer. For example:
+
+.. code-block:: c++
+
+  [[clang::nomerge]] void (*foo)(int);
+
+  void bar(int x) {
+    auto *ptr = foo;
+    if (x) foo(1); else foo(2); // will not be merged
+    if (x) ptr(1); else ptr(2); // 'ptr' has no 'nomerge' attribute, can be merged
+  }
   }];
 }
 
 def NoInlineDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
 This function attribute suppresses the inlining of a function at the call sites
 of the function.
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

def warn_auto_var_is_id : Warning<
   InGroup<DiagGroup<"auto-var-id">>;
 
 // Attributes
 def warn_attribute_ignored_no_calls_in_stmt: Warning<
   "%0 attribute is ignored because there exists no call expression inside the "
   "statement">,
   InGroup<IgnoredAttributes>;
 
+def warn_attribute_ignored_non_function_pointer: Warning<
+  "%0 attribute is ignored because %1 is not a function pointer">,
+  InGroup<IgnoredAttributes>;
+
 def warn_function_attribute_ignored_in_stmt : Warning<
   "attribute is ignored on this statement as it only applies to functions; "
   "use '%0' on statements">,
   InGroup<IgnoredAttributes>;
 
 def err_musttail_needs_trivial_args : Error<
   "tail call requires that the return value, all parameters, and any "
   "temporaries created by the expression are trivially destructible">;

clang/lib/CodeGen/CGCall.cpp

if (const FunctionDecl *Fn = dyn_cast<FunctionDecl>(TargetDecl)) {
       const bool IsVirtualCall = MD && MD->isVirtual();
       // Don't use [[noreturn]], _Noreturn or [[no_builtin]] for a call to a
       // virtual function. These attributes are not inherited by overloads.
       if (!(AttrOnCallSite && IsVirtualCall)) {
         if (Fn->isNoReturn())
           FuncAttrs.addAttribute(llvm::Attribute::NoReturn);
         NBA = Fn->getAttr<NoBuiltinAttr>();
       }
+    }
+
+    if (isa<FunctionDecl>(TargetDecl) || isa<VarDecl>(TargetDecl)) {
       // Only place nomerge attribute on call sites, never functions. This
       // allows it to work on indirect virtual function calls.
       if (AttrOnCallSite && TargetDecl->hasAttr<NoMergeAttr>())
         FuncAttrs.addAttribute(llvm::Attribute::NoMerge);
     }
 
     // 'const', 'pure' and 'noalias' attributed functions are also nounwind.
     if (TargetDecl->hasAttr<ConstAttr>()) {

clang/lib/Sema/SemaDeclAttr.cpp

 }
 
 static void handleAvailableOnlyInDefaultEvalMethod(Sema &S, Decl *D,
                                                    const ParsedAttr &AL) {
   assert(isa<TypedefNameDecl>(D) && "This attribute only applies to a typedef");
   handleSimpleAttribute<AvailableOnlyInDefaultEvalMethodAttr>(S, D, AL);
 }
 
+static void handleNoMergeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  auto *VDecl = dyn_cast<VarDecl>(D);
+  if (VDecl && !VDecl->isFunctionPointerType()) {
+    S.Diag(AL.getLoc(), diag::warn_attribute_ignored_non_function_pointer)
+        << AL << VDecl;
+    return;
+  }
+  D->addAttr(NoMergeAttr::Create(S.Context, AL));
+}
+
 static void handleSYCLKernelAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   // The 'sycl_kernel' attribute applies only to function templates.
   const auto *FD = cast<FunctionDecl>(D);
   const FunctionTemplateDecl *FT = FD->getDescribedFunctionTemplate();
   assert(FT && "Function template is expected");
 
   // Function template must have at least two template parameters.
   const TemplateParameterList *TL = FT->getTemplateParameters();
case ParsedAttr::AT_InternalLinkage:
     handleInternalLinkageAttr(S, D, AL);
     break;
   case ParsedAttr::AT_ZeroCallUsedRegs:
     handleZeroCallUsedRegsAttr(S, D, AL);
     break;
   case ParsedAttr::AT_FunctionReturnThunks:
     handleFunctionReturnThunksAttr(S, D, AL);
     break;
+  case ParsedAttr::AT_NoMerge:
+    handleNoMergeAttr(S, D, AL);
+    break;
 
   case ParsedAttr::AT_AvailableOnlyInDefaultEvalMethod:
     handleAvailableOnlyInDefaultEvalMethod(S, D, AL);
     break;
 
   // Microsoft attributes:
   case ParsedAttr::AT_LayoutVersion:
     handleLayoutVersion(S, D, AL);

clang/test/CodeGen/attr-nomerge.cpp

 
 class B : public A {
 public:
   void g() override;
 };
 
 bool bar();
 [[clang::nomerge]] void f(bool, bool);
+[[clang::nomerge]] void (*fptr)(void);
 
 void foo(int i, A *ap, B *bp) {
   [[clang::nomerge]] bar();
   [[clang::nomerge]] (i = 4, bar());
   [[clang::nomerge]] (void)(bar());
   f(bar(), bar());
+  fptr();
   [[clang::nomerge]] [] { bar(); bar(); }(); // nomerge only applies to the anonymous function call
   [[clang::nomerge]] for (bar(); bar(); bar()) {}
   [[clang::nomerge]] { asm("nop"); }
   bar();
 
   ap->g();
   bp->g();
 

 }
 
 // CHECK: call noundef zeroext i1 @_Z3barv() #[[ATTR0:[0-9]+]]
 // CHECK: call noundef zeroext i1 @_Z3barv() #[[ATTR0]]
 // CHECK: call noundef zeroext i1 @_Z3barv() #[[ATTR0]]
 // CHECK: call noundef zeroext i1 @_Z3barv(){{$}}
 // CHECK: call noundef zeroext i1 @_Z3barv(){{$}}
 // CHECK: call void @_Z1fbb({{.*}}) #[[ATTR0]]
+// CHECK: %[[FPTR:.*]] = load ptr, ptr @fptr
+// CHECK-NEXT: call void %[[FPTR]]() #[[ATTR0]]
 // CHECK: call void @"_ZZ3fooiP1AP1BENK3$_0clEv"{{.*}} #[[ATTR0]]
 // CHECK: call noundef zeroext i1 @_Z3barv() #[[ATTR0]]
 // CHECK-LABEL: for.cond:
 // CHECK: call noundef zeroext i1 @_Z3barv() #[[ATTR0]]
 // CHECK-LABEL: for.inc:
 // CHECK: call noundef zeroext i1 @_Z3barv() #[[ATTR0]]
 // CHECK: call void asm sideeffect "nop"{{.*}} #[[ATTR1:[0-9]+]]
 // CHECK: call noundef zeroext i1 @_Z3barv(){{$}}

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

 // CHECK-NEXT: NoBuiltin (SubjectMatchRule_function)
 // CHECK-NEXT: NoCommon (SubjectMatchRule_variable)
 // CHECK-NEXT: NoDebug (SubjectMatchRule_type_alias, SubjectMatchRule_hasType_functionType, SubjectMatchRule_objc_method, SubjectMatchRule_variable_not_is_parameter)
 // CHECK-NEXT: NoDestroy (SubjectMatchRule_variable)
 // CHECK-NEXT: NoDuplicate (SubjectMatchRule_function)
 // CHECK-NEXT: NoEscape (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: NoInline (SubjectMatchRule_function)
 // CHECK-NEXT: NoInstrumentFunction (SubjectMatchRule_function, SubjectMatchRule_objc_method)
-// CHECK-NEXT: NoMerge (SubjectMatchRule_function)
+// CHECK-NEXT: NoMerge (SubjectMatchRule_function, SubjectMatchRule_variable)
 // CHECK-NEXT: NoMicroMips (SubjectMatchRule_function)
 // CHECK-NEXT: NoMips16 (SubjectMatchRule_function)
 // CHECK-NEXT: NoProfileFunction (SubjectMatchRule_function)
 // CHECK-NEXT: NoRandomizeLayout (SubjectMatchRule_record)
 // CHECK-NEXT: NoSanitize (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: NoSanitizeSpecific (SubjectMatchRule_function, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: NoSpeculativeLoadHardening (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: NoSplitStack (SubjectMatchRule_function)

clang/test/Parser/stmt-attributes.c

void foobar(void) {
   __attribute__((nomerge)) bar();
   __attribute__(()) bar();                // expected-error {{expected identifier or '('}}
   __attribute__((unused, nomerge)) bar(); // expected-error {{expected identifier or '('}}
   __attribute__((nomerge, unused)) bar(); // expected-error {{expected identifier or '('}}
   __attribute__((nomerge(1, 2))) bar();   // expected-error {{'nomerge' attribute takes no arguments}}
   int x;
   __attribute__((nomerge)) x = 10; // expected-warning {{'nomerge' attribute is ignored because there exists no call expression inside the statement}}
 
-  __attribute__((nomerge)) label : bar(); // expected-error {{'nomerge' attribute only applies to functions and statements}}
+  __attribute__((nomerge)) label : bar(); // expected-error {{'nomerge' attribute only applies to functions, statements and variables}}
 }
 
 int f(void);
 
-__attribute__((nomerge)) static int i; // expected-error {{'nomerge' attribute only applies to functions and statements}}
+__attribute__((nomerge)) static int (*fptr)(void);
+__attribute__((nomerge)) static int i; // expected-warning {{'nomerge' attribute is ignored because 'i' is not a function pointer}}
+struct buz {} __attribute__((nomerge)); // expected-error {{'nomerge' attribute only applies to functions, statements and variables}}

clang/test/Parser/stmt-attributes.cpp

 // RUN: %clang_cc1 -std=c++11 -fcxx-exceptions -fexceptions -fsyntax-only -verify %s
 
 #if !__has_extension(statement_attributes_with_gnu_syntax)
 #error "We should have statement attributes with GNU syntax support"
 #endif
 
 template <typename T = void>
 class __attribute__((nomerge)) A {
-  // expected-error@-1 {{'nomerge' attribute only applies to functions and statements}}
+  // expected-error@-1 {{'nomerge' attribute only applies to functions, statements and variables}}
 };
 
 class B : public A<> {
 public:
   void bar();
 };
 
 void bar();

clang/test/Parser/stmt-attributes.m

 - (void)foo;
 - (void)bar;
 @end
 
 Test *getTest(void);
 
 @implementation Test
 - (void)foo __attribute__((nomerge)) {
-  // expected-error@-1 {{'nomerge' attribute only applies to functions and statements}}
+  // expected-error@-1 {{'nomerge' attribute only applies to functions, statements and variables}}
 }
 
 - (void)bar {
   __attribute__(()) [self foo];
   // expected-error@-1 {{missing '[' at start of message send expression}}
   // expected-error@-2 {{expected ']'}}
   // expected-error@-3 {{expected identifier or '('}}
   // expected-note@-4 {{to match this '['}}

clang/test/Sema/attr-nomerge-ast.cpp

 // RUN: %clang_cc1 -ast-dump %s 2>&1 | FileCheck %s
 
 [[clang::nomerge]] void func();
 [[clang::nomerge]] void func();
 void func();
 [[clang::nomerge]] void func() {}
+[[clang::nomerge]] void (*var)(void);
 
 // CHECK: FunctionDecl {{.*}} func 'void ()'
 // CHECK-NEXT: NoMergeAttr
 // CHECK-NEXT: FunctionDecl {{.*}} func 'void ()'
 // CHECK-NEXT: NoMergeAttr
 // CHECK-NEXT: FunctionDecl {{.*}} func 'void ()'
 // CHECK-NEXT: NoMergeAttr {{.*}} Inherited
 // CHECK-NEXT: FunctionDecl {{.*}} func 'void ()'
 // CHECK-NEXT: CompoundStmt
 // CHECK-NEXT: NoMergeAttr
+
+// CHECK-NEXT: VarDecl {{.*}} var 'void (*)()'
+// CHECK-NEXT: NoMergeAttr

clang/test/Sema/attr-nomerge.cpp

 // RUN: %clang_cc1 -verify -fsyntax-only %s
 
 void bar();
 
 void foo() {
   [[clang::nomerge]] bar();
   [[clang::nomerge(1, 2)]] bar(); // expected-error {{'nomerge' attribute takes no arguments}}
   int x;
   [[clang::nomerge]] x = 10; // expected-warning {{'nomerge' attribute is ignored because there exists no call expression inside the statement}}
 
-  [[clang::nomerge]] label: bar(); // expected-error {{'nomerge' attribute only applies to functions and statements}}
+  [[clang::nomerge]] label: bar(); // expected-error {{'nomerge' attribute only applies to functions, statements and variables}}
 
 }
 
 [[clang::nomerge]] int f();
 
-[[clang::nomerge]] static int i = f(); // expected-error {{'nomerge' attribute only applies to functions and statements}}
+[[clang::nomerge]] static int i = f(); // expected-warning {{'nomerge' attribute is ignored because 'i' is not a function pointer}}
+
+[[clang::nomerge]] void (*j)(void);
+
+struct [[clang::nomerge]] buz {}; // expected-error {{'nomerge' attribute only applies to functions, statements and variables}}