[Clang][Codegen] emit noprofile IR Fn Attr for no_instrument_function Fn Attr

[nickdesaulniers]

Summary

noprofile IR attribute already exists to prevent profiling with PGO;
emit that when a function uses the no_instrument_function function
attribute.

The Linux kernel would like to avoid compiler generated code in
functions annotated with such attribute. We already respect this for
libcalls to fentry() and mcount().

Link: https://lore.kernel.org/lkml/[e-mail]/
Signed-off-by: Nick Desaulniers <[e-mail]>

[nickdesaulniers]

• add -disable-llvm-passes to unit test

[void]

What are your thoughts on adding a noprofile function attribute in the FE? @MaskRay suggested filing a bug with gcov (below) to get their take on it.

https://gcc.gnu.org/bugzilla/buglist.cgi?bug_status=__open__&component=gcov-profile&list_id=304970&product=gcc

[nickdesaulniers]

• rename new test from fprofile-generate.c to fprofile-instrument.c; -fprofile-generate is the driver opt, -fprofile-instrument is the frontend opt.

[melver]

In D104253#2817673, @void wrote:

What are your thoughts on adding a noprofile function attribute in the FE? @MaskRay suggested filing a bug with gcov (below) to get their take on it.

https://gcc.gnu.org/bugzilla/buglist.cgi?bug_status=__open__&component=gcov-profile&list_id=304970&product=gcc

How similar is all this stuff? Looking at https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html

If no_instrument_function is the catchall for this profile/gcov/pg instrumentation, I don't think anybody will object.

The result is 1) pg/-fprofile/gcov stuff, 2) sanitizers. (1) is suppressible via no_instrument_function, (2) via no_sanitize*. This appears consistent vs. having no_instrument_function+noprofile+no_sanitize*

As a user, this is the intuitive behaviour I'd expect at least.

But asking GCC folks (Martin helped us with no_sanitize_coverage) for input is reasonable.

clang/test/CodeGen/fprofile-instrument.c

5 ↗

(On Diff #351960)

Only one of the spellings should be required.

[MaskRay]

__attribute__((no_instrument_function)) seems specific to -finstrument-functions. Somehow gcc -pg uses it as well.

The name may not be generic, so it may be odd to exclude various instrumentations under this generic attribute.

clang -fprofile-instr-generate (frontend coverage/PGO; AIUI only Apples uses it for PGO) / clang -fprofile-generate (IR PGO; most users use this) / clang -fprofile-arcs / gcc -fprofile-arcs are 4 different instrumentation approaches.
Their user-facing options are similar enough and the instrumented output has similarity (so a user not wanting one instr will certainly not want other instr) so I think they warrant similar function attribute.

Perhaps a good idea to file a feature request on https://gcc.gnu.org/bugzilla/buglist.cgi?bug_status=__open__&component=gcov-profile&list_id=304970&product=gcc and check what function attributes GCC wants to use.

[nickdesaulniers]

In D104253#2817695, @MaskRay wrote:

__attribute__((no_instrument_function)) seems specific to -finstrument-functions. Somehow gcc -pg uses it as well.

The name may not be generic, so it may be odd to exclude various instrumentations under this generic attribute.

clang -fprofile-instr-generate (frontend coverage/PGO; AIUI only Apples uses it for PGO) / clang -fprofile-generate (IR PGO; most users use this) / clang -fprofile-arcs / gcc -fprofile-arcs are 4 different instrumentation approaches.
Their user-facing options are similar enough and the instrumented output has similarity (so a user not wanting one instr will certainly not want other instr) so I think they warrant similar function attribute.

Perhaps a good idea to file a feature request on https://gcc.gnu.org/bugzilla/buglist.cgi?bug_status=__open__&component=gcov-profile&list_id=304970&product=gcc and check what function attributes GCC wants to use.

Done; thanks for the advice: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223#c6.

[MaskRay]

For the function attributing disabling instrumentation of -fprofile-generate/-fprofile-instr-generate/-fcs-profile-generate/-fprofile-arcs, how about no_profile?

[nickdesaulniers]

• fixup double fn attr in test as per @melver.

[nickdesaulniers]

Jotting down driver to front end flag translations:

-fprofile-generate -> -fprofile-instrument=llvm
-fprofile-instr-generate -> -fprofile-instrument=clang
-fcs-profile-generate -> -fprofile-instrument=csllvm
-fprofile-arcs -> -fprofile-arcs

In D104253#2819995, @MaskRay wrote:

For the function attributing disabling instrumentation of -fprofile-generate/-fprofile-instr-generate/-fcs-profile-generate/-fprofile-arcs, how about no_profile?

Sure, abandoning. Prefer D104475.

clang/lib/CodeGen/CodeGenFunction.cpp

if (CGM.getCodeGenOpts().ProfileSampleAccurate)
     Fn->addFnAttr("profile-sample-accurate");
 
   if (!CGM.getCodeGenOpts().SampleProfileFile.empty())
     Fn->addFnAttr("use-sample-profile");
 
   if (D && D->hasAttr<CFICanonicalJumpTableAttr>())
     Fn->addFnAttr("cfi-canonical-jump-table");
 
+  if (D && D->hasAttr<NoInstrumentFunctionAttr>())
+    Fn->addFnAttr(llvm::Attribute::NoProfile);
+
   if (getLangOpts().OpenCL) {
     // Add metadata for a kernel function.
     if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D))
       EmitOpenCLKernelMetadata(FD, Fn);
   }
 
   // If we are checking function types, emit a function type signature as
   // prologue data.

clang/test/CodeGen/fprofile-generate.c

+// RUN: %clang_cc1 -fprofile-instrument=llvm -disable-llvm-passes \
+// RUN:   -emit-llvm -o - %s | FileCheck %s
+int g(int);
+
+int __attribute__((__no_instrument_function__))
+__attribute__((no_instrument_function))
+no_instr(int a) {
+// CHECK: define {{.*}} i32 @no_instr(i32 %a) [[ATTR:#[0-9]+]]
+  int sum = 0;
+  for (int i = 0; i < a; i++)
+    sum += g(i);
+  return sum;
+}
+
+int instr(int a) {
+// CHECK: define {{.*}} i32 @instr(i32 %a) [[ATTR2:#[0-9]+]]
+  int sum = 0;
+  for (int i = 0; i < a; i++)
+    sum += g(i);
+  return sum;
+}
+// CHECK: attributes [[ATTR]] = {{.*}} noprofile
+// CHECK-NOT: attributes [[ATTR2]] = {{.*}} noprofile