Properly mark lambdas in GCOV (PR gcov-profile/86109).

Message ID 32c23cc0-f311-682c-ecb4-ee7777d8ede8@suse.cz
State New
Headers show
Series
  • Properly mark lambdas in GCOV (PR gcov-profile/86109).
Related show

Commit Message

Martin Liška Sept. 12, 2018, 12:39 p.m.
Hi.

This is follow-up of:
https://gcc.gnu.org/ml/gcc/2018-08/msg00007.html

I've chosen to implement that with new DECL_CXX_LAMBDA_FUNCTION that
uses an empty bit in tree_function_decl.

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready for trunk?

gcc/ChangeLog:

2018-09-12  Martin Liska  <mliska@suse.cz>

	PR gcov-profile/86109
	* coverage.c (coverage_begin_function): Do not
	mark lambdas as artificial.
	* tree-core.h (struct GTY): Remove tm_clone_flag
	and introduce new lambda_function.
	* tree.h (DECL_CXX_LAMBDA_FUNCTION): New macro.

gcc/cp/ChangeLog:

2018-09-12  Martin Liska  <mliska@suse.cz>

	PR gcov-profile/86109
	* parser.c (cp_parser_lambda_declarator_opt):
	Set DECL_CXX_LAMBDA_FUNCTION for lambdas.

gcc/testsuite/ChangeLog:

2018-09-12  Martin Liska  <mliska@suse.cz>

	PR gcov-profile/86109
	* g++.dg/gcov/pr86109.C: New test.
---
 gcc/coverage.c                      |  3 ++-
 gcc/cp/parser.c                     |  1 +
 gcc/testsuite/g++.dg/gcov/pr86109.C | 16 ++++++++++++++++
 gcc/tree-core.h                     |  2 +-
 gcc/tree.h                          |  4 ++++
 5 files changed, 24 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/gcov/pr86109.C

Comments

Martin Liška Sept. 21, 2018, 9:04 a.m. | #1
PING^1

On 9/12/18 2:39 PM, Martin Liška wrote:
> Hi.

> 

> This is follow-up of:

> https://gcc.gnu.org/ml/gcc/2018-08/msg00007.html

> 

> I've chosen to implement that with new DECL_CXX_LAMBDA_FUNCTION that

> uses an empty bit in tree_function_decl.

> 

> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

> 

> Ready for trunk?

> 

> gcc/ChangeLog:

> 

> 2018-09-12  Martin Liska  <mliska@suse.cz>

> 

> 	PR gcov-profile/86109

> 	* coverage.c (coverage_begin_function): Do not

> 	mark lambdas as artificial.

> 	* tree-core.h (struct GTY): Remove tm_clone_flag

> 	and introduce new lambda_function.

> 	* tree.h (DECL_CXX_LAMBDA_FUNCTION): New macro.

> 

> gcc/cp/ChangeLog:

> 

> 2018-09-12  Martin Liska  <mliska@suse.cz>

> 

> 	PR gcov-profile/86109

> 	* parser.c (cp_parser_lambda_declarator_opt):

> 	Set DECL_CXX_LAMBDA_FUNCTION for lambdas.

> 

> gcc/testsuite/ChangeLog:

> 

> 2018-09-12  Martin Liska  <mliska@suse.cz>

> 

> 	PR gcov-profile/86109

> 	* g++.dg/gcov/pr86109.C: New test.

> ---

>  gcc/coverage.c                      |  3 ++-

>  gcc/cp/parser.c                     |  1 +

>  gcc/testsuite/g++.dg/gcov/pr86109.C | 16 ++++++++++++++++

>  gcc/tree-core.h                     |  2 +-

>  gcc/tree.h                          |  4 ++++

>  5 files changed, 24 insertions(+), 2 deletions(-)

>  create mode 100644 gcc/testsuite/g++.dg/gcov/pr86109.C

> 

>
Jeff Law Oct. 2, 2018, 3:32 p.m. | #2
On 9/12/18 6:39 AM, Martin Liška wrote:
> Hi.

> 

> This is follow-up of:

> https://gcc.gnu.org/ml/gcc/2018-08/msg00007.html

> 

> I've chosen to implement that with new DECL_CXX_LAMBDA_FUNCTION that

> uses an empty bit in tree_function_decl.

> 

> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

> 

> Ready for trunk?

> 

> gcc/ChangeLog:

> 

> 2018-09-12  Martin Liska  <mliska@suse.cz>

> 

> 	PR gcov-profile/86109

> 	* coverage.c (coverage_begin_function): Do not

> 	mark lambdas as artificial.

> 	* tree-core.h (struct GTY): Remove tm_clone_flag

> 	and introduce new lambda_function.

> 	* tree.h (DECL_CXX_LAMBDA_FUNCTION): New macro.

> 

> gcc/cp/ChangeLog:

> 

> 2018-09-12  Martin Liska  <mliska@suse.cz>

> 

> 	PR gcov-profile/86109

> 	* parser.c (cp_parser_lambda_declarator_opt):

> 	Set DECL_CXX_LAMBDA_FUNCTION for lambdas.

> 

> gcc/testsuite/ChangeLog:

> 

> 2018-09-12  Martin Liska  <mliska@suse.cz>

> 

> 	PR gcov-profile/86109

> 	* g++.dg/gcov/pr86109.C: New test.

So the concern here is C++-isms bleeding into the language independent
nodes.  I think a name change from DECL_CXX_LAMBDA_FUNCTION to something
else would be enough to go forward.

jeff
Martin Liška Oct. 2, 2018, 5:14 p.m. | #3
On 10/2/18 5:32 PM, Jeff Law wrote:
> On 9/12/18 6:39 AM, Martin Liška wrote:

>> Hi.

>>

>> This is follow-up of:

>> https://gcc.gnu.org/ml/gcc/2018-08/msg00007.html

>>

>> I've chosen to implement that with new DECL_CXX_LAMBDA_FUNCTION that

>> uses an empty bit in tree_function_decl.

>>

>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

>>

>> Ready for trunk?

>>

>> gcc/ChangeLog:

>>

>> 2018-09-12  Martin Liska  <mliska@suse.cz>

>>

>> 	PR gcov-profile/86109

>> 	* coverage.c (coverage_begin_function): Do not

>> 	mark lambdas as artificial.

>> 	* tree-core.h (struct GTY): Remove tm_clone_flag

>> 	and introduce new lambda_function.

>> 	* tree.h (DECL_CXX_LAMBDA_FUNCTION): New macro.

>>

>> gcc/cp/ChangeLog:

>>

>> 2018-09-12  Martin Liska  <mliska@suse.cz>

>>

>> 	PR gcov-profile/86109

>> 	* parser.c (cp_parser_lambda_declarator_opt):

>> 	Set DECL_CXX_LAMBDA_FUNCTION for lambdas.

>>

>> gcc/testsuite/ChangeLog:

>>

>> 2018-09-12  Martin Liska  <mliska@suse.cz>

>>

>> 	PR gcov-profile/86109

>> 	* g++.dg/gcov/pr86109.C: New test.


Hi.

Thanks for the review.

> So the concern here is C++-isms bleeding into the language independent

> nodes.  I think a name change from DECL_CXX_LAMBDA_FUNCTION to something

> else would be enough to go forward.


Agree, well, then I would suggest to use DECL_LAMBDA_FUNCTION. The concept
of lambda functions is quite common in other programming languages.

Martin

> 

> jeff

>
Jeff Law Oct. 2, 2018, 5:15 p.m. | #4
On 10/2/18 11:14 AM, Martin Liška wrote:
> On 10/2/18 5:32 PM, Jeff Law wrote:

>> On 9/12/18 6:39 AM, Martin Liška wrote:

>>> Hi.

>>>

>>> This is follow-up of:

>>> https://gcc.gnu.org/ml/gcc/2018-08/msg00007.html

>>>

>>> I've chosen to implement that with new DECL_CXX_LAMBDA_FUNCTION that

>>> uses an empty bit in tree_function_decl.

>>>

>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression

>>> tests.

>>>

>>> Ready for trunk?

>>>

>>> gcc/ChangeLog:

>>>

>>> 2018-09-12  Martin Liska  <mliska@suse.cz>

>>>

>>>     PR gcov-profile/86109

>>>     * coverage.c (coverage_begin_function): Do not

>>>     mark lambdas as artificial.

>>>     * tree-core.h (struct GTY): Remove tm_clone_flag

>>>     and introduce new lambda_function.

>>>     * tree.h (DECL_CXX_LAMBDA_FUNCTION): New macro.

>>>

>>> gcc/cp/ChangeLog:

>>>

>>> 2018-09-12  Martin Liska  <mliska@suse.cz>

>>>

>>>     PR gcov-profile/86109

>>>     * parser.c (cp_parser_lambda_declarator_opt):

>>>     Set DECL_CXX_LAMBDA_FUNCTION for lambdas.

>>>

>>> gcc/testsuite/ChangeLog:

>>>

>>> 2018-09-12  Martin Liska  <mliska@suse.cz>

>>>

>>>     PR gcov-profile/86109

>>>     * g++.dg/gcov/pr86109.C: New test.

> 

> Hi.

> 

> Thanks for the review.

> 

>> So the concern here is C++-isms bleeding into the language independent

>> nodes.  I think a name change from DECL_CXX_LAMBDA_FUNCTION to something

>> else would be enough to go forward.

> 

> Agree, well, then I would suggest to use DECL_LAMBDA_FUNCTION. The concept

> of lambda functions is quite common in other programming languages.

Agreed and OK with that change.

jeff

Patch

diff --git a/gcc/coverage.c b/gcc/coverage.c
index bae6f5cafac..550d84598ab 100644
--- a/gcc/coverage.c
+++ b/gcc/coverage.c
@@ -657,7 +657,8 @@  coverage_begin_function (unsigned lineno_checksum, unsigned cfg_checksum)
   gcov_write_string (IDENTIFIER_POINTER
 		     (DECL_ASSEMBLER_NAME (current_function_decl)));
   gcov_write_unsigned (DECL_ARTIFICIAL (current_function_decl)
-		       && !DECL_FUNCTION_VERSIONED (current_function_decl));
+		       && !DECL_FUNCTION_VERSIONED (current_function_decl)
+		       && !DECL_CXX_LAMBDA_FUNCTION (current_function_decl));
   gcov_write_filename (xloc.file);
   gcov_write_unsigned (xloc.line);
   gcov_write_unsigned (xloc.column);
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index f5e4fa4ff07..a1d4e30a966 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -10649,6 +10649,7 @@  cp_parser_lambda_declarator_opt (cp_parser* parser, tree lambda_expr)
 	DECL_ARTIFICIAL (fco) = 1;
 	/* Give the object parameter a different name.  */
 	DECL_NAME (DECL_ARGUMENTS (fco)) = closure_identifier;
+	DECL_CXX_LAMBDA_FUNCTION (fco) = 1;
       }
     if (template_param_list)
       {
diff --git a/gcc/testsuite/g++.dg/gcov/pr86109.C b/gcc/testsuite/g++.dg/gcov/pr86109.C
new file mode 100644
index 00000000000..9052d2e5a04
--- /dev/null
+++ b/gcc/testsuite/g++.dg/gcov/pr86109.C
@@ -0,0 +1,16 @@ 
+
+/* { dg-options "-fprofile-arcs -ftest-coverage -std=c++11" } */
+/* { dg-do run { target native } } */
+
+int main()
+{
+    auto partially_uncovered_lambda = [](int i) { /* count(1) */
+        if (i > 10) /* count(1) */
+            return 0; /* count(1) */
+        return 1; /* count(#####) */
+    };
+
+    return partially_uncovered_lambda(20); /* count(1) */
+}
+
+/* { dg-final { run-gcov pr86109.C } } */
diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index dee27f89dec..cd3a2bad08c 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -1789,8 +1789,8 @@  struct GTY(()) tree_function_decl {
   unsigned pure_flag : 1;
   unsigned looping_const_or_pure_flag : 1;
   unsigned has_debug_args_flag : 1;
-  unsigned tm_clone_flag : 1;
   unsigned versioned_function : 1;
+  unsigned lambda_function: 1;
   /* No bits left.  */
 };
 
diff --git a/gcc/tree.h b/gcc/tree.h
index 4f415b7a220..692123eafe3 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -3039,6 +3039,10 @@  extern vec<tree, va_gc> **decl_debug_args_insert (tree);
 #define DECL_CXX_DESTRUCTOR_P(NODE)\
    (FUNCTION_DECL_CHECK (NODE)->decl_with_vis.cxx_destructor)
 
+/* In FUNCTION_DECL, this is set if this function is a C++ lambda function.  */
+#define DECL_CXX_LAMBDA_FUNCTION(NODE) \
+  (FUNCTION_DECL_CHECK (NODE)->function_decl.lambda_function)
+
 /* In FUNCTION_DECL that represent an virtual method this is set when
    the method is final.  */
 #define DECL_FINAL_P(NODE)\