[PATCH] D104253: [Clang][Codegen] emit noprofile IR Fn Attr for no_instrument_function Fn Attr

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers abandoned this revision.
nickdesaulniers added a comment.

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 .


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104253/new/

https://reviews.llvm.org/D104253

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104253: [Clang][Codegen] emit noprofile IR Fn Attr for no_instrument_function Fn Attr

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 352208.
nickdesaulniers added a comment.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104253/new/

https://reviews.llvm.org/D104253

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/fprofile-instrument.c


Index: clang/test/CodeGen/fprofile-instrument.c
===
--- /dev/null
+++ clang/test/CodeGen/fprofile-instrument.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -fprofile-instrument=llvm -disable-llvm-passes \
+// RUN:   -emit-llvm -o - %s | FileCheck %s
+int g(int);
+
+int __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
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -893,6 +893,9 @@
   if (D && D->hasAttr())
 Fn->addFnAttr("cfi-canonical-jump-table");
 
+  if (D && D->hasAttr())
+Fn->addFnAttr(llvm::Attribute::NoProfile);
+
   if (getLangOpts().OpenCL) {
 // Add metadata for a kernel function.
 if (const FunctionDecl *FD = dyn_cast_or_null(D))


Index: clang/test/CodeGen/fprofile-instrument.c
===
--- /dev/null
+++ clang/test/CodeGen/fprofile-instrument.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -fprofile-instrument=llvm -disable-llvm-passes \
+// RUN:   -emit-llvm -o - %s | FileCheck %s
+int g(int);
+
+int __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
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -893,6 +893,9 @@
   if (D && D->hasAttr())
 Fn->addFnAttr("cfi-canonical-jump-table");
 
+  if (D && D->hasAttr())
+Fn->addFnAttr(llvm::Attribute::NoProfile);
+
   if (getLangOpts().OpenCL) {
 // Add metadata for a kernel function.
 if (const FunctionDecl *FD = dyn_cast_or_null(D))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104253: [Clang][Codegen] emit noprofile IR Fn Attr for no_instrument_function Fn Attr

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104253/new/

https://reviews.llvm.org/D104253

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104253: [Clang][Codegen] emit noprofile IR Fn Attr for no_instrument_function Fn Attr

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

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__=gcov-profile_id=304970=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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104253/new/

https://reviews.llvm.org/D104253

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104253: [Clang][Codegen] emit noprofile IR Fn Attr for no_instrument_function Fn Attr

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

`__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,
I think they are similar enough that warrant a similar function attribute.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104253/new/

https://reviews.llvm.org/D104253

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104253: [Clang][Codegen] emit noprofile IR Fn Attr for no_instrument_function Fn Attr

[Marco Elver via Phabricator via cfe-commits]

melver added a comment.

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__=gcov-profile_id=304970=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.




Comment at: clang/test/CodeGen/fprofile-instrument.c:5
+
+int __attribute__((__no_instrument_function__))
+__attribute__((no_instrument_function))

Only one of the spellings should be required.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104253/new/

https://reviews.llvm.org/D104253

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104253: [Clang][Codegen] emit noprofile IR Fn Attr for no_instrument_function Fn Attr

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 351960.
nickdesaulniers added a comment.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104253/new/

https://reviews.llvm.org/D104253

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/fprofile-instrument.c


Index: clang/test/CodeGen/fprofile-instrument.c
===
--- /dev/null
+++ clang/test/CodeGen/fprofile-instrument.c
@@ -0,0 +1,23 @@
+// 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
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -893,6 +893,9 @@
   if (D && D->hasAttr())
 Fn->addFnAttr("cfi-canonical-jump-table");
 
+  if (D && D->hasAttr())
+Fn->addFnAttr(llvm::Attribute::NoProfile);
+
   if (getLangOpts().OpenCL) {
 // Add metadata for a kernel function.
 if (const FunctionDecl *FD = dyn_cast_or_null(D))


Index: clang/test/CodeGen/fprofile-instrument.c
===
--- /dev/null
+++ clang/test/CodeGen/fprofile-instrument.c
@@ -0,0 +1,23 @@
+// 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
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -893,6 +893,9 @@
   if (D && D->hasAttr())
 Fn->addFnAttr("cfi-canonical-jump-table");
 
+  if (D && D->hasAttr())
+Fn->addFnAttr(llvm::Attribute::NoProfile);
+
   if (getLangOpts().OpenCL) {
 // Add metadata for a kernel function.
 if (const FunctionDecl *FD = dyn_cast_or_null(D))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104253: [Clang][Codegen] emit noprofile IR Fn Attr for no_instrument_function Fn Attr

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

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__=gcov-profile_id=304970=gcc


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104253/new/

https://reviews.llvm.org/D104253

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104253: [Clang][Codegen] emit noprofile IR Fn Attr for no_instrument_function Fn Attr

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 351956.
nickdesaulniers added a comment.

- add -disable-llvm-passes to unit test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104253/new/

https://reviews.llvm.org/D104253

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/fprofile-generate.c


Index: clang/test/CodeGen/fprofile-generate.c
===
--- /dev/null
+++ clang/test/CodeGen/fprofile-generate.c
@@ -0,0 +1,23 @@
+// 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
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -893,6 +893,9 @@
   if (D && D->hasAttr())
 Fn->addFnAttr("cfi-canonical-jump-table");
 
+  if (D && D->hasAttr())
+Fn->addFnAttr(llvm::Attribute::NoProfile);
+
   if (getLangOpts().OpenCL) {
 // Add metadata for a kernel function.
 if (const FunctionDecl *FD = dyn_cast_or_null(D))


Index: clang/test/CodeGen/fprofile-generate.c
===
--- /dev/null
+++ clang/test/CodeGen/fprofile-generate.c
@@ -0,0 +1,23 @@
+// 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
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -893,6 +893,9 @@
   if (D && D->hasAttr())
 Fn->addFnAttr("cfi-canonical-jump-table");
 
+  if (D && D->hasAttr())
+Fn->addFnAttr(llvm::Attribute::NoProfile);
+
   if (getLangOpts().OpenCL) {
 // Add metadata for a kernel function.
 if (const FunctionDecl *FD = dyn_cast_or_null(D))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104253: [Clang][Codegen] emit noprofile IR Fn Attr for no_instrument_function Fn Attr

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added reviewers: MaskRay, melver, void.
Herald added a subscriber: wenlei.
nickdesaulniers requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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/cakwvodmpti93n2l0_yqkrzldmpxzror7zggszonyaw2pgsh...@mail.gmail.com/
Signed-off-by: Nick Desaulniers 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104253

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/fprofile-generate.c


Index: clang/test/CodeGen/fprofile-generate.c
===
--- /dev/null
+++ clang/test/CodeGen/fprofile-generate.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fprofile-instrument=llvm -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
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -893,6 +893,9 @@
   if (D && D->hasAttr())
 Fn->addFnAttr("cfi-canonical-jump-table");
 
+  if (D && D->hasAttr())
+Fn->addFnAttr(llvm::Attribute::NoProfile);
+
   if (getLangOpts().OpenCL) {
 // Add metadata for a kernel function.
 if (const FunctionDecl *FD = dyn_cast_or_null(D))


Index: clang/test/CodeGen/fprofile-generate.c
===
--- /dev/null
+++ clang/test/CodeGen/fprofile-generate.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fprofile-instrument=llvm -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
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -893,6 +893,9 @@
   if (D && D->hasAttr())
 Fn->addFnAttr("cfi-canonical-jump-table");
 
+  if (D && D->hasAttr())
+Fn->addFnAttr(llvm::Attribute::NoProfile);
+
   if (getLangOpts().OpenCL) {
 // Add metadata for a kernel function.
 if (const FunctionDecl *FD = dyn_cast_or_null(D))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits