[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list=<arg1, arg2, ...> to match gcc options.

[kchoi via Phabricator via cfe-commits]

choikwa updated this revision to Diff 120252.
choikwa added a comment.

rebase to trunk


https://reviews.llvm.org/D37624

Files:
  docs/ClangCommandLineReference.rst
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/instrument-functions.c
  test/CodeGenCXX/instrument-functions.cpp

Index: test/CodeGenCXX/instrument-functions.cpp
===
--- test/CodeGenCXX/instrument-functions.cpp
+++ test/CodeGenCXX/instrument-functions.cpp
@@ -1,10 +1,32 @@
 // RUN: %clang_cc1 -S -emit-llvm -triple %itanium_abi_triple -o - %s -finstrument-functions | FileCheck %s
 
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions | FileCheck %s
+// RUN: %clang -S -emit-llvm -o - %s -fno-instrument-functions | FileCheck %s --check-prefix=NOINSTR
+
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-file-list=instrument | FileCheck %s --check-prefix=NOFILE
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=test3 | FileCheck %s --check-prefix=NOFUNC
+
+// Below would see if mangled name partially matches. exclude-function-list matches demangled names, thus we expect to see instrument calls in test3.
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=Z5test3 | FileCheck %s --check-prefix=NOFUNC2
+
+
 // CHECK: @_Z5test1i
+// NOINSTR: @_Z5test1i
+// NOFILE: @_Z5test1i
+// NOFUNC: @_Z5test1i
 int test1(int x) {
 // CHECK: __cyg_profile_func_enter
 // CHECK: __cyg_profile_func_exit
 // CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC: __cyg_profile_func_enter
+// NOFUNC: __cyg_profile_func_exit
+// NOFUNC: ret
   return x;
 }
 
@@ -17,6 +39,93 @@
   return x;
 }
 
+// CHECK: @_Z5test3i
+// NOINSTR: @_Z5test3i
+// NOFILE: @_Z5test3i
+// NOFUNC: @_Z5test3i
+// NOFUNC2: @_Z5test3i
+int test3(int x) {
+// CHECK: __cyg_profile_func_enter
+// CHECK: __cyg_profile_func_exit
+// CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC-NOT: __cyg_profile_func_enter
+// NOFUNC-NOT: __cyg_profile_func_exit
+// NOFUNC: ret
+// NOFUNC2: __cyg_profile_func_enter
+// NOFUNC2: __cyg_profile_func_exit
+// NOFUNC2: ret
+  return x;
+}
+
+// Check function overload
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=test3 | FileCheck %s --check-prefix=OVERLOAD
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=Z5test3v | FileCheck %s --check-prefix=OVERLOAD1
+
+// OVERLOAD: @_Z5test3v
+// OVERLOAD1: @_Z5test3v
+int test3() {
+// OVERLOAD-NOT: __cyg_profile_func_enter
+// OVERLOAD-NOT: __cyg_profile_func_exit
+// OVERLOAD: ret
+// OVERLOAD1: __cyg_profile_func_enter
+// OVERLOAD1: __cyg_profile_func_exit
+// OVERLOAD1: ret
+  return 1;
+}
+
+template 
+T test4(T a) {
+  return a;
+}
+
+// Check function with template specialization
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=test4 | FileCheck %s --check-prefix=TPL
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=Z5test4Ii | FileCheck %s --check-prefix=TPL1
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=Z5test4Is | FileCheck %s --check-prefix=TPL2
+
+// TPL: @_Z5test4IiET_S0_
+// TPL1: @_Z5test4IiET_S0_
+template<>
+int test4(int a) {
+// TPL-NOT: __cyg_profile_func_enter
+// TPL-NOT: __cyg_profile_func_exit
+// TPL: ret
+// TPL1: __cyg_profile_func_enter
+// TPL1: __cyg_profile_func_exit
+// TPL1: ret
+  return a;
+}
+
+// TPL: @_Z5test4IsET_S0_
+// TPL2: @_Z5test4IsET_S0_
+template<>
+short test4(short a) {
+// TPL-NOT: __cyg_profile_func_enter
+// TPL-NOT: __cyg_profile_func_exit
+// TPL: ret
+// TPL2: __cyg_profile_func_enter
+// TPL2: __cyg_profile_func_exit
+// TPL2: ret
+  return a;
+}
+
+// Check whitespace
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list='int test5 (float f, int a, int *p)' | FileCheck %s --check-prefix=WSTEST
+// WSTEST: @_Z5test5fiPi
+int test5 (float f, int a, int *p) {
+// WSTEST-NOT: __cyg_profile_func_enter
+// WSTEST-NOT: __cyg_profile_func_exit
+// WSTEST-NOT: ret
+  *p = (int)f + a;
+  return *p;
+}
+
 // This test case previously crashed code generation.  It exists solely
 // to test 

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list=<arg1, arg2, ...> to match gcc options.

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D37624#885438, @choikwa wrote:

> > Can you get more information on what GCC actually implemented and why? It's 
> > not clear to me that ignoring the namespaces is the most-useful way to do 
> > this. I don't want to emulate GCC bugs, but maybe there's a good reason why 
> > their implementation works this way.
>
> This is what I found, https://gcc.gnu.org/ml/gcc-patches/2015-12/msg00473.html


To state the obvious, that patch has zero C++ test cases. Can try emailing the 
author of the feature in GCC and try to get some clarity on what the intended 
behavior is on their end? I'm trying to figure out what's useful here. I think 
we do want to match namespaces.

> Diff shows it doesn't seem to specially treat single quotes.
> 
> +case OPT_finstrument_functions_include_function_list_:
>  +  add_comma_separated_to_vector
>  +(>x_flag_instrument_functions_include_functions, arg);
>  +  break;




https://reviews.llvm.org/D37624



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

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list=<arg1, arg2, ...> to match gcc options.

[kchoi via Phabricator via cfe-commits]

choikwa added a comment.

> Can you get more information on what GCC actually implemented and why? It's 
> not clear to me that ignoring the namespaces is the most-useful way to do 
> this. I don't want to emulate GCC bugs, but maybe there's a good reason why 
> their implementation works this way.

This is what I found, https://gcc.gnu.org/ml/gcc-patches/2015-12/msg00473.html
Diff shows it doesn't seem to specially treat single quotes.

+case OPT_finstrument_functions_include_function_list_:
+  add_comma_separated_to_vector
+   (>x_flag_instrument_functions_include_functions, arg);
+  break;


https://reviews.llvm.org/D37624



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

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list=<arg1, arg2, ...> to match gcc options.

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D37624#885311, @choikwa wrote:

> In https://reviews.llvm.org/D37624#885308, @hfinkel wrote:
>
> > In https://reviews.llvm.org/D37624#885295, @choikwa wrote:
> >
> > > In https://reviews.llvm.org/D37624#885290, @hfinkel wrote:
> > >
> > > > In https://reviews.llvm.org/D37624#885288, @choikwa wrote:
> > > >
> > > > > - add comment to CPP test to explain usage
> > > >
> > > >
> > > > Thanks. Please also add some tests showing matching overloaded 
> > > > functions, functions with template parameters, etc.
> > > >
> > > > Do we need to strip whitespace before trying to match the demangled 
> > > > names?
> > >
> > >
> > > Some cursory testing with g++ shows that only the 'test5' of 
> > > 'test5(float, int, int*)' is matched. 'test5(' or 'test5 (' is not 
> > > matched. It seems weird that arguments are not matched.
> > >
> > > g++ man page shows
> > >
> > >   "The function name to be matched is its user-visible name, such as 
> > > "vector blah(const vector &)", not the internal mangled name"
> > >
> > > but it doesn't seem to be including parameters.
> >
> >
> > Interesting. Can you tell what GCC is doing w.r.t. namespace names, class 
> > names, etc. and template parameters?
> >
> > > Also uncovered a bug where sub argument list containing comma needs to be 
> > > surrounded by single quote, but clang seems to ignores single quote.
> > >  I'll try to dig around ArgList implementation to see if it can return 
> > > argument surrounded by single-quote as a whole.
>
>
> Given A::B::C(T a), only 'C' is meaningful in g++'s matcher. Adding 
> anything else escapes the match [  'B::C', 'C('  ].
>  It seems like g++ will also not match single quotes as a whole, ie. int 
> fooboo() is matched by 'foo,boo'.


Can you get more information on what GCC actually implemented and why? It's not 
clear to me that ignoring the namespaces is the most-useful way to do this. I 
don't want to emulate GCC bugs, but maybe there's a good reason why their 
implementation works this way.


https://reviews.llvm.org/D37624



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

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list=<arg1, arg2, ...> to match gcc options.

[kchoi via Phabricator via cfe-commits]

choikwa added a comment.

In https://reviews.llvm.org/D37624#885308, @hfinkel wrote:

> In https://reviews.llvm.org/D37624#885295, @choikwa wrote:
>
> > In https://reviews.llvm.org/D37624#885290, @hfinkel wrote:
> >
> > > In https://reviews.llvm.org/D37624#885288, @choikwa wrote:
> > >
> > > > - add comment to CPP test to explain usage
> > >
> > >
> > > Thanks. Please also add some tests showing matching overloaded functions, 
> > > functions with template parameters, etc.
> > >
> > > Do we need to strip whitespace before trying to match the demangled names?
> >
> >
> > Some cursory testing with g++ shows that only the 'test5' of 'test5(float, 
> > int, int*)' is matched. 'test5(' or 'test5 (' is not matched. It seems 
> > weird that arguments are not matched.
> >
> > g++ man page shows
> >
> >   "The function name to be matched is its user-visible name, such as 
> > "vector blah(const vector &)", not the internal mangled name"
> >
> > but it doesn't seem to be including parameters.
>
>
> Interesting. Can you tell what GCC is doing w.r.t. namespace names, class 
> names, etc. and template parameters?
>
> > Also uncovered a bug where sub argument list containing comma needs to be 
> > surrounded by single quote, but clang seems to ignores single quote.
> >  I'll try to dig around ArgList implementation to see if it can return 
> > argument surrounded by single-quote as a whole.


Given A::B::C(T a), only 'C' is meaningful in g++'s matcher. Adding 
anything else escapes the match [  'B::C', 'C('  ].


https://reviews.llvm.org/D37624



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

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list=<arg1, arg2, ...> to match gcc options.

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D37624#885295, @choikwa wrote:

> In https://reviews.llvm.org/D37624#885290, @hfinkel wrote:
>
> > In https://reviews.llvm.org/D37624#885288, @choikwa wrote:
> >
> > > - add comment to CPP test to explain usage
> >
> >
> > Thanks. Please also add some tests showing matching overloaded functions, 
> > functions with template parameters, etc.
> >
> > Do we need to strip whitespace before trying to match the demangled names?
>
>
> Some cursory testing with g++ shows that only the 'test5' of 'test5(float, 
> int, int*)' is matched. 'test5(' or 'test5 (' is not matched. It seems weird 
> that arguments are not matched.
>
> g++ man page shows
>
>   "The function name to be matched is its user-visible name, such as 
> "vector blah(const vector &)", not the internal mangled name"
>
> but it doesn't seem to be including parameters.


Interesting. Can you tell what GCC is doing w.r.t. namespace names, class 
names, etc. and template parameters?

> Also uncovered a bug where sub argument list containing comma needs to be 
> surrounded by single quote, but clang seems to ignores single quote.
>  I'll try to dig around ArgList implementation to see if it can return 
> argument surrounded by single-quote as a whole.




https://reviews.llvm.org/D37624



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

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list=<arg1, arg2, ...> to match gcc options.

[kchoi via Phabricator via cfe-commits]

choikwa added a comment.

In https://reviews.llvm.org/D37624#885290, @hfinkel wrote:

> In https://reviews.llvm.org/D37624#885288, @choikwa wrote:
>
> > - add comment to CPP test to explain usage
>
>
> Thanks. Please also add some tests showing matching overloaded functions, 
> functions with template parameters, etc.
>
> Do we need to strip whitespace before trying to match the demangled names?


Some cursory testing with g++ shows that only the 'test5' of 'test5(float, int, 
int*)' is matched. 'test5(' or 'test5 (' is not matched. It seems weird that 
arguments are not matched.




Comment at: test/CodeGenCXX/instrument-functions.cpp:8
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions 
-finstrument-functions-exclude-function-list=test3 | FileCheck %s 
--check-prefix=NOFUNC
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions 
-finstrument-functions-exclude-function-list=Z5test3 | FileCheck %s 
--check-prefix=NOFUNC2
+

hfinkel wrote:
> I'm a bit confused about this test. Are you trying to match against the 
> mangled names, or the demangled names, or both?
It's matching demangled names, so Z5test3 would not match and insert 
instrumentation calls to test3(int). Since this wasn't clear, I will add 
comment in the test.


https://reviews.llvm.org/D37624



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

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list=<arg1, arg2, ...> to match gcc options.

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: lib/CodeGen/CodeGenFunction.cpp:455
+for (const auto  : PathSearch) {
+  if(FunctionDeclPath.find(FileMatch) != std::string::npos) {
+return false;

Space after if.



Comment at: lib/CodeGen/CodeGenFunction.cpp:473
+  for (const auto  : FunctionSearch) {
+if(FunctionName.find(FuncMatch) != std::string::npos) {
+  return false;

Ditto.


https://reviews.llvm.org/D37624



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

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list=<arg1, arg2, ...> to match gcc options.

[kchoi via Phabricator via cfe-commits]

choikwa updated this revision to Diff 117269.
choikwa added a comment.

- add more CPP tests: func overload, template special


https://reviews.llvm.org/D37624

Files:
  docs/ClangCommandLineReference.rst
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/instrument-functions.c
  test/CodeGenCXX/instrument-functions.cpp

Index: test/CodeGenCXX/instrument-functions.cpp
===
--- test/CodeGenCXX/instrument-functions.cpp
+++ test/CodeGenCXX/instrument-functions.cpp
@@ -1,10 +1,32 @@
 // RUN: %clang_cc1 -S -emit-llvm -triple %itanium_abi_triple -o - %s -finstrument-functions | FileCheck %s
 
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions | FileCheck %s
+// RUN: %clang -S -emit-llvm -o - %s -fno-instrument-functions | FileCheck %s --check-prefix=NOINSTR
+
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-file-list=instrument | FileCheck %s --check-prefix=NOFILE
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=test3 | FileCheck %s --check-prefix=NOFUNC
+
+// Below would see if mangled name partially matches. exclude-function-list matches demangled names, thus we expect to see instrument calls in test3.
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=Z5test3 | FileCheck %s --check-prefix=NOFUNC2
+
+
 // CHECK: @_Z5test1i
+// NOINSTR: @_Z5test1i
+// NOFILE: @_Z5test1i
+// NOFUNC: @_Z5test1i
 int test1(int x) {
 // CHECK: __cyg_profile_func_enter
 // CHECK: __cyg_profile_func_exit
 // CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC: __cyg_profile_func_enter
+// NOFUNC: __cyg_profile_func_exit
+// NOFUNC: ret
   return x;
 }
 
@@ -17,6 +39,82 @@
   return x;
 }
 
+// CHECK: @_Z5test3i
+// NOINSTR: @_Z5test3i
+// NOFILE: @_Z5test3i
+// NOFUNC: @_Z5test3i
+// NOFUNC2: @_Z5test3i
+int test3(int x) {
+// CHECK: __cyg_profile_func_enter
+// CHECK: __cyg_profile_func_exit
+// CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC-NOT: __cyg_profile_func_enter
+// NOFUNC-NOT: __cyg_profile_func_exit
+// NOFUNC: ret
+// NOFUNC2: __cyg_profile_func_enter
+// NOFUNC2: __cyg_profile_func_exit
+// NOFUNC2: ret
+  return x;
+}
+
+// Check function overload
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=test3 | FileCheck %s --check-prefix=OVERLOAD
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=Z5test3v | FileCheck %s --check-prefix=OVERLOAD1
+
+// OVERLOAD: @_Z5test3v
+// OVERLOAD1: @_Z5test3v
+int test3() {
+// OVERLOAD-NOT: __cyg_profile_func_enter
+// OVERLOAD-NOT: __cyg_profile_func_exit
+// OVERLOAD: ret
+// OVERLOAD1: __cyg_profile_func_enter
+// OVERLOAD1: __cyg_profile_func_exit
+// OVERLOAD1: ret
+  return 1;
+}
+
+template 
+T test4(T a) {
+  return a;
+}
+
+// Check function with template specialization
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=test4 | FileCheck %s --check-prefix=TPL
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=Z5test4Ii | FileCheck %s --check-prefix=TPL1
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=Z5test4Is | FileCheck %s --check-prefix=TPL2
+
+// TPL: @_Z5test4IiET_S0_
+// TPL1: @_Z5test4IiET_S0_
+template<>
+int test4(int a) {
+// TPL-NOT: __cyg_profile_func_enter
+// TPL-NOT: __cyg_profile_func_exit
+// TPL: ret
+// TPL1: __cyg_profile_func_enter
+// TPL1: __cyg_profile_func_exit
+// TPL1: ret
+  return a;
+}
+
+// TPL: @_Z5test4IsET_S0_
+// TPL2: @_Z5test4IsET_S0_
+template<>
+short test4(short a) {
+// TPL-NOT: __cyg_profile_func_enter
+// TPL-NOT: __cyg_profile_func_exit
+// TPL: ret
+// TPL2: __cyg_profile_func_enter
+// TPL2: __cyg_profile_func_exit
+// TPL2: ret
+  return a;
+}
+
 // This test case previously crashed code generation.  It exists solely
 // to test -finstrument-function does not crash codegen for this trivial
 // case.
Index: test/CodeGen/instrument-functions.c
===
--- test/CodeGen/instrument-functions.c
+++ test/CodeGen/instrument-functions.c
@@ -1,18 +1,66 @@
-// RUN: %clang_cc1 -S -debug-info-kind=standalone -emit-llvm -o - %s -finstrument-functions | 

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list=<arg1, arg2, ...> to match gcc options.

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D37624#885288, @choikwa wrote:

> - add comment to CPP test to explain usage


Thanks. Please also add some tests showing matching overloaded functions, 
functions with template parameters, etc.

Do we need to strip whitespace before trying to match the demangled names?


https://reviews.llvm.org/D37624



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

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list=<arg1, arg2, ...> to match gcc options.

[kchoi via Phabricator via cfe-commits]

choikwa updated this revision to Diff 117268.
choikwa added a comment.

- add comment to CPP test to explain usage


https://reviews.llvm.org/D37624

Files:
  docs/ClangCommandLineReference.rst
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/instrument-functions.c
  test/CodeGenCXX/instrument-functions.cpp

Index: test/CodeGenCXX/instrument-functions.cpp
===
--- test/CodeGenCXX/instrument-functions.cpp
+++ test/CodeGenCXX/instrument-functions.cpp
@@ -1,10 +1,32 @@
 // RUN: %clang_cc1 -S -emit-llvm -triple %itanium_abi_triple -o - %s -finstrument-functions | FileCheck %s
 
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions | FileCheck %s
+// RUN: %clang -S -emit-llvm -o - %s -fno-instrument-functions | FileCheck %s --check-prefix=NOINSTR
+
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-file-list=instrument | FileCheck %s --check-prefix=NOFILE
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=test3 | FileCheck %s --check-prefix=NOFUNC
+
+// Below would see if mangled name partially matches. exclude-function-list matches demangled names, thus we expect to see instrument calls in test3.
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=Z5test3 | FileCheck %s --check-prefix=NOFUNC2
+
+
 // CHECK: @_Z5test1i
+// NOINSTR: @_Z5test1i
+// NOFILE: @_Z5test1i
+// NOFUNC: @_Z5test1i
 int test1(int x) {
 // CHECK: __cyg_profile_func_enter
 // CHECK: __cyg_profile_func_exit
 // CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC: __cyg_profile_func_enter
+// NOFUNC: __cyg_profile_func_exit
+// NOFUNC: ret
   return x;
 }
 
@@ -17,6 +39,30 @@
   return x;
 }
 
+// CHECK: @_Z5test3i
+// NOINSTR: @_Z5test3i
+// NOFILE: @_Z5test3i
+// NOFUNC: @_Z5test3i
+// NOFUNC2: @_Z5test3i
+int test3(int x){
+// CHECK: __cyg_profile_func_enter
+// CHECK: __cyg_profile_func_exit
+// CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC-NOT: __cyg_profile_func_enter
+// NOFUNC-NOT: __cyg_profile_func_exit
+// NOFUNC: ret
+// NOFUNC2: __cyg_profile_func_enter
+// NOFUNC2: __cyg_profile_func_exit
+// NOFUNC2: ret
+  return x;
+}
+
 // This test case previously crashed code generation.  It exists solely
 // to test -finstrument-function does not crash codegen for this trivial
 // case.
Index: test/CodeGen/instrument-functions.c
===
--- test/CodeGen/instrument-functions.c
+++ test/CodeGen/instrument-functions.c
@@ -1,18 +1,66 @@
-// RUN: %clang_cc1 -S -debug-info-kind=standalone -emit-llvm -o - %s -finstrument-functions | FileCheck %s
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions | FileCheck %s
+// RUN: %clang -S -emit-llvm -o - %s -fno-instrument-functions | FileCheck %s --check-prefix=NOINSTR
+
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-file-list=instrument | FileCheck %s --check-prefix=NOFILE
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=test3 | FileCheck %s --check-prefix=NOFUNC
 
 // CHECK: @test1
+// NOINSTR: @test1
+// NOFILE: @test1
+// NOFUNC: @test1
 int test1(int x) {
-// CHECK: call void @__cyg_profile_func_enter({{.*}}, !dbg
-// CHECK: call void @__cyg_profile_func_exit({{.*}}, !dbg
+// CHECK: __cyg_profile_func_enter
+// CHECK: __cyg_profile_func_exit
 // CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC: __cyg_profile_func_enter
+// NOFUNC: __cyg_profile_func_exit
+// NOFUNC: ret
   return x;
 }
 
 // CHECK: @test2
+// NOINSTR: @test2
+// NOFILE: @test2
+// NOFUNC: @test2
 int test2(int) __attribute__((no_instrument_function));
 int test2(int x) {
 // CHECK-NOT: __cyg_profile_func_enter
 // CHECK-NOT: __cyg_profile_func_exit
 // CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC-NOT: __cyg_profile_func_enter
+// NOFUNC-NOT: __cyg_profile_func_exit
+// NOFUNC: ret
+  return x;
+}
+
+// CHECK: @test3
+// NOINSTR: @test3
+// NOFILE: @test3
+// NOFUNC: 

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list=<arg1, arg2, ...> to match gcc options.

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added inline comments.



Comment at: test/CodeGenCXX/instrument-functions.cpp:8
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions 
-finstrument-functions-exclude-function-list=test3 | FileCheck %s 
--check-prefix=NOFUNC
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions 
-finstrument-functions-exclude-function-list=Z5test3 | FileCheck %s 
--check-prefix=NOFUNC2
+

I'm a bit confused about this test. Are you trying to match against the mangled 
names, or the demangled names, or both?


https://reviews.llvm.org/D37624



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

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list=<arg1, arg2, ...> to match gcc options.

[kchoi via Phabricator via cfe-commits]

choikwa updated this revision to Diff 117267.
choikwa added a comment.

- - Address formating feedback, remove redundant inline


https://reviews.llvm.org/D37624

Files:
  docs/ClangCommandLineReference.rst
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/instrument-functions.c
  test/CodeGenCXX/instrument-functions.cpp

Index: test/CodeGenCXX/instrument-functions.cpp
===
--- test/CodeGenCXX/instrument-functions.cpp
+++ test/CodeGenCXX/instrument-functions.cpp
@@ -1,10 +1,29 @@
 // RUN: %clang_cc1 -S -emit-llvm -triple %itanium_abi_triple -o - %s -finstrument-functions | FileCheck %s
 
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions | FileCheck %s
+// RUN: %clang -S -emit-llvm -o - %s -fno-instrument-functions | FileCheck %s --check-prefix=NOINSTR
+
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-file-list=instrument | FileCheck %s --check-prefix=NOFILE
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=test3 | FileCheck %s --check-prefix=NOFUNC
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=Z5test3 | FileCheck %s --check-prefix=NOFUNC2
+
 // CHECK: @_Z5test1i
+// NOINSTR: @_Z5test1i
+// NOFILE: @_Z5test1i
+// NOFUNC: @_Z5test1i
 int test1(int x) {
 // CHECK: __cyg_profile_func_enter
 // CHECK: __cyg_profile_func_exit
 // CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC: __cyg_profile_func_enter
+// NOFUNC: __cyg_profile_func_exit
+// NOFUNC: ret
   return x;
 }
 
@@ -17,6 +36,30 @@
   return x;
 }
 
+// CHECK: @_Z5test3i
+// NOINSTR: @_Z5test3i
+// NOFILE: @_Z5test3i
+// NOFUNC: @_Z5test3i
+// NOFUNC2: @_Z5test3i
+int test3(int x){
+// CHECK: __cyg_profile_func_enter
+// CHECK: __cyg_profile_func_exit
+// CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC-NOT: __cyg_profile_func_enter
+// NOFUNC-NOT: __cyg_profile_func_exit
+// NOFUNC: ret
+// NOFUNC2: __cyg_profile_func_enter
+// NOFUNC2: __cyg_profile_func_exit
+// NOFUNC2: ret
+  return x;
+}
+
 // This test case previously crashed code generation.  It exists solely
 // to test -finstrument-function does not crash codegen for this trivial
 // case.
Index: test/CodeGen/instrument-functions.c
===
--- test/CodeGen/instrument-functions.c
+++ test/CodeGen/instrument-functions.c
@@ -1,18 +1,66 @@
-// RUN: %clang_cc1 -S -debug-info-kind=standalone -emit-llvm -o - %s -finstrument-functions | FileCheck %s
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions | FileCheck %s
+// RUN: %clang -S -emit-llvm -o - %s -fno-instrument-functions | FileCheck %s --check-prefix=NOINSTR
+
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-file-list=instrument | FileCheck %s --check-prefix=NOFILE
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=test3 | FileCheck %s --check-prefix=NOFUNC
 
 // CHECK: @test1
+// NOINSTR: @test1
+// NOFILE: @test1
+// NOFUNC: @test1
 int test1(int x) {
-// CHECK: call void @__cyg_profile_func_enter({{.*}}, !dbg
-// CHECK: call void @__cyg_profile_func_exit({{.*}}, !dbg
+// CHECK: __cyg_profile_func_enter
+// CHECK: __cyg_profile_func_exit
 // CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC: __cyg_profile_func_enter
+// NOFUNC: __cyg_profile_func_exit
+// NOFUNC: ret
   return x;
 }
 
 // CHECK: @test2
+// NOINSTR: @test2
+// NOFILE: @test2
+// NOFUNC: @test2
 int test2(int) __attribute__((no_instrument_function));
 int test2(int x) {
 // CHECK-NOT: __cyg_profile_func_enter
 // CHECK-NOT: __cyg_profile_func_exit
 // CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC-NOT: __cyg_profile_func_enter
+// NOFUNC-NOT: __cyg_profile_func_exit
+// NOFUNC: ret
+  return x;
+}
+
+// CHECK: @test3
+// NOINSTR: @test3
+// NOFILE: @test3
+// NOFUNC: @test3
+int test3(int x) {
+// CHECK: __cyg_profile_func_enter
+// CHECK: __cyg_profile_func_exit
+// CHECK: ret
+// NOINSTR-NOT: 

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list=<arg1, arg2, ...> to match gcc options.

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: lib/CodeGen/CodeGenFunction.cpp:463
+
+  // Skip demangling if decl is extern "C"
+  if (ActualFuncDecl && !ActualFuncDecl->isExternC()) {

Is this comment still correct?



Comment at: lib/CodeGen/CodeGenModule.h:503
+  /// Mapping from SourceLocation to PresumedLoc FileName
+  llvm::DenseMap SourceLocToFileNameMap;
+

Pointers lean right.



Comment at: lib/CodeGen/CodeGenModule.h:1212
+  /// Get SourceLoc to FileName map cache
+  inline llvm::DenseMap () {
+return SourceLocToFileNameMap;

inline is redundant here.


https://reviews.llvm.org/D37624



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

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list=<arg1, arg2, ...> to match gcc options.

[kchoi via Phabricator via cfe-commits]

choikwa updated this revision to Diff 117264.
choikwa added a comment.

Addressing Hal's feedback


https://reviews.llvm.org/D37624

Files:
  docs/ClangCommandLineReference.rst
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/instrument-functions.c
  test/CodeGenCXX/instrument-functions.cpp

Index: test/CodeGenCXX/instrument-functions.cpp
===
--- test/CodeGenCXX/instrument-functions.cpp
+++ test/CodeGenCXX/instrument-functions.cpp
@@ -1,10 +1,28 @@
 // RUN: %clang_cc1 -S -emit-llvm -triple %itanium_abi_triple -o - %s -finstrument-functions | FileCheck %s
 
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions | FileCheck %s
+// RUN: %clang -S -emit-llvm -o - %s -fno-instrument-functions | FileCheck %s --check-prefix=NOINSTR
+
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-file-list=instrument | FileCheck %s --check-prefix=NOFILE
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=test3 | FileCheck %s --check-prefix=NOFUNC
+
 // CHECK: @_Z5test1i
+// NOINSTR: @_Z5test1i
+// NOFILE: @_Z5test1i
+// NOFUNC: @_Z5test1i
 int test1(int x) {
 // CHECK: __cyg_profile_func_enter
 // CHECK: __cyg_profile_func_exit
 // CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC: __cyg_profile_func_enter
+// NOFUNC: __cyg_profile_func_exit
+// NOFUNC: ret
   return x;
 }
 
@@ -17,6 +35,26 @@
   return x;
 }
 
+// CHECK: @_Z5test3i
+// NOINSTR: @_Z5test3i
+// NOFILE: @_Z5test3i
+// NOFUNC: @_Z5test3i
+int test3(int x){
+// CHECK: __cyg_profile_func_enter
+// CHECK: __cyg_profile_func_exit
+// CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC-NOT: __cyg_profile_func_enter
+// NOFUNC-NOT: __cyg_profile_func_exit
+// NOFUNC: ret
+  return x;
+}
+
 // This test case previously crashed code generation.  It exists solely
 // to test -finstrument-function does not crash codegen for this trivial
 // case.
Index: test/CodeGen/instrument-functions.c
===
--- test/CodeGen/instrument-functions.c
+++ test/CodeGen/instrument-functions.c
@@ -1,18 +1,66 @@
-// RUN: %clang_cc1 -S -debug-info-kind=standalone -emit-llvm -o - %s -finstrument-functions | FileCheck %s
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions | FileCheck %s
+// RUN: %clang -S -emit-llvm -o - %s -fno-instrument-functions | FileCheck %s --check-prefix=NOINSTR
+
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-file-list=instrument | FileCheck %s --check-prefix=NOFILE
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=test3 | FileCheck %s --check-prefix=NOFUNC
 
 // CHECK: @test1
+// NOINSTR: @test1
+// NOFILE: @test1
+// NOFUNC: @test1
 int test1(int x) {
-// CHECK: call void @__cyg_profile_func_enter({{.*}}, !dbg
-// CHECK: call void @__cyg_profile_func_exit({{.*}}, !dbg
+// CHECK: __cyg_profile_func_enter
+// CHECK: __cyg_profile_func_exit
 // CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC: __cyg_profile_func_enter
+// NOFUNC: __cyg_profile_func_exit
+// NOFUNC: ret
   return x;
 }
 
 // CHECK: @test2
+// NOINSTR: @test2
+// NOFILE: @test2
+// NOFUNC: @test2
 int test2(int) __attribute__((no_instrument_function));
 int test2(int x) {
 // CHECK-NOT: __cyg_profile_func_enter
 // CHECK-NOT: __cyg_profile_func_exit
 // CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC-NOT: __cyg_profile_func_enter
+// NOFUNC-NOT: __cyg_profile_func_exit
+// NOFUNC: ret
+  return x;
+}
+
+// CHECK: @test3
+// NOINSTR: @test3
+// NOFILE: @test3
+// NOFUNC: @test3
+int test3(int x) {
+// CHECK: __cyg_profile_func_enter
+// CHECK: __cyg_profile_func_exit
+// CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC-NOT: __cyg_profile_func_enter
+// NOFUNC-NOT: __cyg_profile_func_exit
+// NOFUNC: ret
   return x;
 }
Index: 

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list=<arg1, arg2, ...> to match gcc options.

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

Please also add a C++ test to check the mangling-related features.




Comment at: lib/CodeGen/CodeGenFunction.cpp:419
+// Assume that __cxa_demangle is provided by libcxxabi (except for Windows).
+extern "C" char *__cxa_demangle(const char *mangled_name, char *output_buffer,
+size_t *length, int *status);

Using the system demangler here seems like the wrong solution. You have the 
current function declaration, `CurFuncDecl`, and from it you should be able to 
extract the full name.

I think that this will do it:

  std::string FullFuncName = PredefinedExpr::ComputeName(
  PredefinedExpr::PrettyFunctionNoVirtual, CurFuncDecl);




Comment at: lib/CodeGen/CodeGenModule.cpp:4601
+CodeGenModule::GetSourceLocToFileNameMap() {
+  return SourceLocToFileNameMap;
+}

Make this function inline in the class definition.


https://reviews.llvm.org/D37624



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

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list=<arg1, arg2, ...> to match gcc options.

[kchoi via Phabricator via cfe-commits]

choikwa updated this revision to Diff 114388.
choikwa added a comment.

renamed and moved Cache to SourceLocToFileNameMap in CodeGenModule


https://reviews.llvm.org/D37624

Files:
  docs/ClangCommandLineReference.rst
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/instrument-functions.c

Index: test/CodeGen/instrument-functions.c
===
--- test/CodeGen/instrument-functions.c
+++ test/CodeGen/instrument-functions.c
@@ -1,18 +1,66 @@
-// RUN: %clang_cc1 -S -debug-info-kind=standalone -emit-llvm -o - %s -finstrument-functions | FileCheck %s
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions | FileCheck %s
+// RUN: %clang -S -emit-llvm -o - %s -fno-instrument-functions | FileCheck %s --check-prefix=NOINSTR
+
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-file-list=instrument | FileCheck %s --check-prefix=NOFILE
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=test3 | FileCheck %s --check-prefix=NOFUNC
 
 // CHECK: @test1
+// NOINSTR: @test1
+// NOFILE: @test1
+// NOFUNC: @test1
 int test1(int x) {
-// CHECK: call void @__cyg_profile_func_enter({{.*}}, !dbg
-// CHECK: call void @__cyg_profile_func_exit({{.*}}, !dbg
+// CHECK: __cyg_profile_func_enter
+// CHECK: __cyg_profile_func_exit
 // CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC: __cyg_profile_func_enter
+// NOFUNC: __cyg_profile_func_exit
+// NOFUNC: ret
   return x;
 }
 
 // CHECK: @test2
+// NOINSTR: @test2
+// NOFILE: @test2
+// NOFUNC: @test2
 int test2(int) __attribute__((no_instrument_function));
 int test2(int x) {
 // CHECK-NOT: __cyg_profile_func_enter
 // CHECK-NOT: __cyg_profile_func_exit
 // CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC-NOT: __cyg_profile_func_enter
+// NOFUNC-NOT: __cyg_profile_func_exit
+// NOFUNC: ret
+  return x;
+}
+
+// CHECK: @test3
+// NOINSTR: @test3
+// NOFILE: @test3
+// NOFUNC: @test3
+int test3(int x) {
+// CHECK: __cyg_profile_func_enter
+// CHECK: __cyg_profile_func_exit
+// CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC-NOT: __cyg_profile_func_enter
+// NOFUNC-NOT: __cyg_profile_func_exit
+// NOFUNC: ret
   return x;
 }
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -771,6 +771,12 @@
 
   Opts.PreserveVec3Type = Args.hasArg(OPT_fpreserve_vec3_type);
   Opts.InstrumentFunctions = Args.hasArg(OPT_finstrument_functions);
+  if (Opts.InstrumentFunctions) {
+Opts.InstrumentFunctionExclusionsFunctions
+= Args.getAllArgValues(OPT_finstrument_functions_exclude_function_list);
+Opts.InstrumentFunctionExclusionsPathSegments
+= Args.getAllArgValues(OPT_finstrument_functions_exclude_file_list);
+  }
   Opts.XRayInstrumentFunctions = Args.hasArg(OPT_fxray_instrument);
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3519,7 +3519,14 @@
 options::OPT_fno_unique_section_names, true))
 CmdArgs.push_back("-fno-unique-section-names");
 
-  Args.AddAllArgs(CmdArgs, options::OPT_finstrument_functions);
+  if (Args.hasArg(options::OPT_finstrument_functions,
+  options::OPT_fno_instrument_functions, false)) {
+Args.AddAllArgs(CmdArgs, options::OPT_finstrument_functions);
+Args.AddAllArgs(CmdArgs,
+options::OPT_finstrument_functions_exclude_file_list);
+Args.AddAllArgs(CmdArgs,
+options::OPT_finstrument_functions_exclude_function_list);
+  }
 
   addPGOAndCoverageFlags(C, D, Output, Args, CmdArgs);
 
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -499,6 +499,9 @@
   /// MDNodes.
   llvm::DenseMap MetadataIdMap;
 
+  /// Mapping from SourceLocation to PresumedLoc 

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list=<arg1, arg2, ...> to match gcc options.

[kchoi via Phabricator via cfe-commits]

choikwa added a comment.

Forgot to hang Cache to CodeGenModule, will do that shortly


https://reviews.llvm.org/D37624



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

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list=<arg1, arg2, ...> to match gcc options.

[kchoi via Phabricator via cfe-commits]

choikwa updated this revision to Diff 114380.
choikwa added a comment.

addressed code review. made doc consistent with functionality.


https://reviews.llvm.org/D37624

Files:
  docs/ClangCommandLineReference.rst
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/instrument-functions.c

Index: test/CodeGen/instrument-functions.c
===
--- test/CodeGen/instrument-functions.c
+++ test/CodeGen/instrument-functions.c
@@ -1,18 +1,66 @@
-// RUN: %clang_cc1 -S -debug-info-kind=standalone -emit-llvm -o - %s -finstrument-functions | FileCheck %s
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions | FileCheck %s
+// RUN: %clang -S -emit-llvm -o - %s -fno-instrument-functions | FileCheck %s --check-prefix=NOINSTR
+
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-file-list=instrument | FileCheck %s --check-prefix=NOFILE
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=test3 | FileCheck %s --check-prefix=NOFUNC
 
 // CHECK: @test1
+// NOINSTR: @test1
+// NOFILE: @test1
+// NOFUNC: @test1
 int test1(int x) {
-// CHECK: call void @__cyg_profile_func_enter({{.*}}, !dbg
-// CHECK: call void @__cyg_profile_func_exit({{.*}}, !dbg
+// CHECK: __cyg_profile_func_enter
+// CHECK: __cyg_profile_func_exit
 // CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC: __cyg_profile_func_enter
+// NOFUNC: __cyg_profile_func_exit
+// NOFUNC: ret
   return x;
 }
 
 // CHECK: @test2
+// NOINSTR: @test2
+// NOFILE: @test2
+// NOFUNC: @test2
 int test2(int) __attribute__((no_instrument_function));
 int test2(int x) {
 // CHECK-NOT: __cyg_profile_func_enter
 // CHECK-NOT: __cyg_profile_func_exit
 // CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC-NOT: __cyg_profile_func_enter
+// NOFUNC-NOT: __cyg_profile_func_exit
+// NOFUNC: ret
+  return x;
+}
+
+// CHECK: @test3
+// NOINSTR: @test3
+// NOFILE: @test3
+// NOFUNC: @test3
+int test3(int x) {
+// CHECK: __cyg_profile_func_enter
+// CHECK: __cyg_profile_func_exit
+// CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC-NOT: __cyg_profile_func_enter
+// NOFUNC-NOT: __cyg_profile_func_exit
+// NOFUNC: ret
   return x;
 }
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -771,6 +771,12 @@
 
   Opts.PreserveVec3Type = Args.hasArg(OPT_fpreserve_vec3_type);
   Opts.InstrumentFunctions = Args.hasArg(OPT_finstrument_functions);
+  if (Opts.InstrumentFunctions) {
+Opts.InstrumentFunctionExclusionsFunctions
+= Args.getAllArgValues(OPT_finstrument_functions_exclude_function_list);
+Opts.InstrumentFunctionExclusionsPathSegments
+= Args.getAllArgValues(OPT_finstrument_functions_exclude_file_list);
+  }
   Opts.XRayInstrumentFunctions = Args.hasArg(OPT_fxray_instrument);
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3519,7 +3519,14 @@
 options::OPT_fno_unique_section_names, true))
 CmdArgs.push_back("-fno-unique-section-names");
 
-  Args.AddAllArgs(CmdArgs, options::OPT_finstrument_functions);
+  if (Args.hasArg(options::OPT_finstrument_functions,
+  options::OPT_fno_instrument_functions, false)) {
+Args.AddAllArgs(CmdArgs, options::OPT_finstrument_functions);
+Args.AddAllArgs(CmdArgs,
+options::OPT_finstrument_functions_exclude_file_list);
+Args.AddAllArgs(CmdArgs,
+options::OPT_finstrument_functions_exclude_function_list);
+  }
 
   addPGOAndCoverageFlags(C, D, Output, Args, CmdArgs);
 
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -1762,7 +1762,7 @@
 
   /// ShouldInstrumentFunction - Return true if the current function should be
   /// instrumented with __cyg_profile_func_* calls
-  bool ShouldInstrumentFunction();
+  bool 

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list=<arg1, arg2, ...> to match gcc options.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/CodeGen/CodeGenFunction.cpp:425
 /// instrumented with __cyg_profile_func_* calls
-bool CodeGenFunction::ShouldInstrumentFunction() {
+bool CodeGenFunction::ShouldInstrumentFunction(llvm::Function *Fn) {
+  typedef std::vector::const_iterator CIt;

This parameter can be `const`.



Comment at: lib/CodeGen/CodeGenFunction.cpp:436
+  // to resolve.
+  const FunctionDecl *ActualFuncDecl = dyn_cast(CurFuncDecl);
+  if (ActualFuncDecl &&

Can use `const auto *` here.



Comment at: lib/CodeGen/CodeGenFunction.cpp:444
+  SourceLocation SLoc = CurFuncDecl->getLocation();
+  static std::unordered_map cache;
+

This static has me worried. Does this data need to be cached across codegen 
modules? If not, perhaps this variable can be hung onto the CodeGenModule 
instead?

The variable should be named `Cache` instead of `cache`. Also, is an 
`unordered_map` the correct data structure to use? Would a `DenseMap` make more 
sense because the keys and values are both small (`PresumedLoc::getFilename()` 
returns a `const char *` that I believe can be used).



Comment at: lib/CodeGen/CodeGenFunction.cpp:447
+  if (SLoc.isFileID()) {
+unsigned key = SLoc.getRawEncoding();
+if (cache.find(key) == cache.end()) {

key -> Key



Comment at: lib/CodeGen/CodeGenFunction.cpp:449
+if (cache.find(key) == cache.end()) {
+  ASTContext  = CurFuncDecl->getASTContext();
+  const SourceManager  = ctx.getSourceManager();

ctx -> CTX, and I think this can be a const ref.



Comment at: lib/CodeGen/CodeGenFunction.cpp:460-461
+
+for (CIt i = PathSearch.begin(), e = PathSearch.end(); i != e; ++i) {
+  if(FunctionDeclPath.find(*i) != std::string::npos) {
+return false;

You can use a range-based for loop here instead, and then get rid of the 
typedef for `CIt`.



Comment at: lib/CodeGen/CodeGenFunction.cpp:467
+
+  std::string FunctionName = Fn->getName();
+

You can avoid the copy here by assigning to a `StringRef` instead.



Comment at: lib/CodeGen/CodeGenFunction.cpp:472
+  if (ActualFuncDecl && !ActualFuncDecl->isExternC()) {
+int status = 0;
+char *result = __cxa_demangle(FunctionName.c_str(), 0, 0, );

status -> Status



Comment at: lib/CodeGen/CodeGenFunction.cpp:473
+int status = 0;
+char *result = __cxa_demangle(FunctionName.c_str(), 0, 0, );
+

result -> Result



Comment at: lib/CodeGen/CodeGenFunction.cpp:486
+CGM.getCodeGenOpts().InstrumentFunctionExclusionsFunctions;
+  for (CIt i = FunctionSearch.begin(), e = FunctionSearch.end(); i != e; ++i) {
+if(FunctionName.find(*i) != std::string::npos) {

Can use a range-based for loop here.


https://reviews.llvm.org/D37624



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

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list=<arg1, arg2, ...> to match gcc options.

[kchoi via Phabricator via cfe-commits]

choikwa created this revision.

Builds on previous Differential https://reviews.llvm.org/D2219

Changes include:

- Using unordered_map with SourceLocation.ID (raw encoding) as key
- Demangle only if !isExternC. Used dyn_cast((Decl*)CurFuncDecl) 
for this
- Modified an existing C testcase to test for options.


https://reviews.llvm.org/D37624

Files:
  docs/ClangCommandLineReference.rst
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/instrument-functions.c

Index: test/CodeGen/instrument-functions.c
===
--- test/CodeGen/instrument-functions.c
+++ test/CodeGen/instrument-functions.c
@@ -1,18 +1,66 @@
-// RUN: %clang_cc1 -S -debug-info-kind=standalone -emit-llvm -o - %s -finstrument-functions | FileCheck %s
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions | FileCheck %s
+// RUN: %clang -S -emit-llvm -o - %s -fno-instrument-functions | FileCheck %s --check-prefix=NOINSTR
+
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-file-list=instrument | FileCheck %s --check-prefix=NOFILE
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions -finstrument-functions-exclude-function-list=test3 | FileCheck %s --check-prefix=NOFUNC
 
 // CHECK: @test1
+// NOINSTR: @test1
+// NOFILE: @test1
+// NOFUNC: @test1
 int test1(int x) {
-// CHECK: call void @__cyg_profile_func_enter({{.*}}, !dbg
-// CHECK: call void @__cyg_profile_func_exit({{.*}}, !dbg
+// CHECK: __cyg_profile_func_enter
+// CHECK: __cyg_profile_func_exit
 // CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC: __cyg_profile_func_enter
+// NOFUNC: __cyg_profile_func_exit
+// NOFUNC: ret
   return x;
 }
 
 // CHECK: @test2
+// NOINSTR: @test2
+// NOFILE: @test2
+// NOFUNC: @test2
 int test2(int) __attribute__((no_instrument_function));
 int test2(int x) {
 // CHECK-NOT: __cyg_profile_func_enter
 // CHECK-NOT: __cyg_profile_func_exit
 // CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC-NOT: __cyg_profile_func_enter
+// NOFUNC-NOT: __cyg_profile_func_exit
+// NOFUNC: ret
+  return x;
+}
+
+// CHECK: @test3
+// NOINSTR: @test3
+// NOFILE: @test3
+// NOFUNC: @test3
+int test3(int x) {
+// CHECK: __cyg_profile_func_enter
+// CHECK: __cyg_profile_func_exit
+// CHECK: ret
+// NOINSTR-NOT: __cyg_profile_func_enter
+// NOINSTR-NOT: __cyg_profile_func_exit
+// NOINSTR: ret
+// NOFILE-NOT: __cyg_profile_func_enter
+// NOFILE-NOT: __cyg_profile_func_exit
+// NOFILE: ret
+// NOFUNC-NOT: __cyg_profile_func_enter
+// NOFUNC-NOT: __cyg_profile_func_exit
+// NOFUNC: ret
   return x;
 }
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -771,6 +771,12 @@
 
   Opts.PreserveVec3Type = Args.hasArg(OPT_fpreserve_vec3_type);
   Opts.InstrumentFunctions = Args.hasArg(OPT_finstrument_functions);
+  if (Opts.InstrumentFunctions) {
+Opts.InstrumentFunctionExclusionsFunctions
+= Args.getAllArgValues(OPT_finstrument_functions_exclude_function_list);
+Opts.InstrumentFunctionExclusionsPathSegments
+= Args.getAllArgValues(OPT_finstrument_functions_exclude_file_list);
+  }
   Opts.XRayInstrumentFunctions = Args.hasArg(OPT_fxray_instrument);
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3519,7 +3519,14 @@
 options::OPT_fno_unique_section_names, true))
 CmdArgs.push_back("-fno-unique-section-names");
 
-  Args.AddAllArgs(CmdArgs, options::OPT_finstrument_functions);
+  if (Args.hasArg(options::OPT_finstrument_functions,
+  options::OPT_fno_instrument_functions, false)) {
+Args.AddAllArgs(CmdArgs, options::OPT_finstrument_functions);
+Args.AddAllArgs(CmdArgs,
+options::OPT_finstrument_functions_exclude_file_list);
+Args.AddAllArgs(CmdArgs,
+options::OPT_finstrument_functions_exclude_function_list);
+  }
 
   addPGOAndCoverageFlags(C, D, Output, Args, CmdArgs);
 
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -1762,7 +1762,7 @@
 
   ///