diff mbox

Fix PR gcov-profile/46266

Message ID 0adbc0be-c219-e1fe-1828-943ddf091fe3@suse.cz
State New
Headers show

Commit Message

Martin Liška Sept. 27, 2016, 11:26 a.m. UTC 
Following patch prevents emission of gcno file (notes file) for statements
that do not point to original source file, like:

$ echo "int main(){}" > x.c

In this case the location points to a builtin.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Martin

Comments

Nathan Sidwell Sept. 27, 2016, 11:32 a.m. UTC | #1 
On 09/27/16 07:26, Martin Liška wrote:
> Following patch prevents emission of gcno file (notes file) for statements
> that do not point to original source file, like:
>
> $ echo "int main(){}" > x.c
>
> In this case the location points to a builtin.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?

I'm sure this is sensible. Is there a difficulty with a testcase -- what exactly 
is the failure mode?

One thing I dislike is negated predicates though -- I think I'd find
    if (!gimple_has_reserved_location (gs))
to be more understandable (particularly as that matches the sense of 
RESERVED_LOCATION_P.

Richard?

nathan
Martin Liška Sept. 27, 2016, 12:45 p.m. UTC | #2 
On 09/27/2016 01:32 PM, Nathan Sidwell wrote:
> On 09/27/16 07:26, Martin Liška wrote:
>> Following patch prevents emission of gcno file (notes file) for statements
>> that do not point to original source file, like:
>>
>> $ echo "int main(){}" > x.c
>>
>> In this case the location points to a builtin.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> Ready to be installed?
> 
> I'm sure this is sensible. Is there a difficulty with a testcase -- what exactly is the failure mode?

Second version of the patch adds validation to gcov.exp, where $result is scanned for "File '<built-in>'".
Luckily current test-case hit that verification:

FAIL: gcc.misc-tests/gcov-6.c gcov failed: <built-in>.gcov should not be created
FAIL: gcc.misc-tests/gcov-7.c gcov failed: <built-in>.gcov should not be created

> 
> One thing I dislike is negated predicates though -- I think I'd find
>    if (!gimple_has_reserved_location (gs))
> to be more understandable (particularly as that matches the sense of RESERVED_LOCATION_P.

Agree with you, renamed to gimple_has_reserved_location.
Ready with that change?

Martin

> 
> Richard?
> 
> nathan
>
diff mbox

Patch

From 0b33d288ecad252d00ef22fd72a1b1f12c0642ff Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Mon, 8 Aug 2016 17:38:07 +0200
Subject: [PATCH] Fix PR gcov-profile/46266

gcc/ChangeLog:

2016-08-12  Martin Liska  <mliska@suse.cz>

	PR gcov-profile/46266
	* gimple.h (gimple_has_not_reserved_location): New function.
	* input.h (RESERVED_LOCATION_P): New macro.
	* profile.c (branch_prob): Use RESERVED_LOCATION_P and
	gimple_has_not_reserved_location instread of comparison
	with UNKNOWN_LOCATION.
---
 gcc/gimple.h  | 8 ++++++++
 gcc/input.h   | 2 ++
 gcc/profile.c | 9 ++++-----
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/gcc/gimple.h b/gcc/gimple.h
index 9fad15b..4982aed 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -1792,6 +1792,14 @@  gimple_has_location (const gimple *g)
   return LOCATION_LOCUS (gimple_location (g)) != UNKNOWN_LOCATION;
 }
 
+/* Return true if G has a location that is not any from reserved.  */
+
+static inline bool
+gimple_has_not_reserved_location (const gimple *g)
+{
+  return !RESERVED_LOCATION_P (gimple_location (g));
+}
+
 
 /* Return the file name of the location of STMT.  */
 
diff --git a/gcc/input.h b/gcc/input.h
index fe80605..6ce0b81 100644
--- a/gcc/input.h
+++ b/gcc/input.h
@@ -61,6 +61,8 @@  extern location_t input_location;
 #define LOCATION_BLOCK(LOC) \
   ((tree) ((IS_ADHOC_LOC (LOC)) ? get_data_from_adhoc_loc (line_table, (LOC)) \
    : NULL))
+#define RESERVED_LOCATION_P(LOC) \
+  (LOCATION_LOCUS (LOC) < RESERVED_LOCATION_COUNT)
 
 /* Return a positive value if LOCATION is the locus of a token that is
    located in a system header, O otherwise. It returns 1 if LOCATION
diff --git a/gcc/profile.c b/gcc/profile.c
index 791225b..ceb5f29 100644
--- a/gcc/profile.c
+++ b/gcc/profile.c
@@ -1042,7 +1042,7 @@  branch_prob (void)
 	       gsi_prev_nondebug (&gsi))
 	    {
 	      last = gsi_stmt (gsi);
-	      if (gimple_has_location (last))
+	      if (gimple_has_not_reserved_location (last))
 		break;
 	    }
 
@@ -1053,7 +1053,7 @@  branch_prob (void)
 	     is not computed twice.  */
 	  if (last
 	      && gimple_has_location (last)
-	      && LOCATION_LOCUS (e->goto_locus) != UNKNOWN_LOCATION
+	      && !RESERVED_LOCATION_P (e->goto_locus)
 	      && !single_succ_p (bb)
 	      && (LOCATION_FILE (e->goto_locus)
 	          != LOCATION_FILE (gimple_location (last))
@@ -1262,15 +1262,14 @@  branch_prob (void)
 	  for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
 	    {
 	      gimple *stmt = gsi_stmt (gsi);
-	      if (gimple_has_location (stmt))
+	      if (gimple_has_not_reserved_location (stmt))
 		output_location (gimple_filename (stmt), gimple_lineno (stmt),
 				 &offset, bb);
 	    }
 
 	  /* Notice GOTO expressions eliminated while constructing the CFG.  */
 	  if (single_succ_p (bb)
-	      && LOCATION_LOCUS (single_succ_edge (bb)->goto_locus)
-		 != UNKNOWN_LOCATION)
+	      && !RESERVED_LOCATION_P (single_succ_edge (bb)->goto_locus))
 	    {
 	      expanded_location curr_location
 		= expand_location (single_succ_edge (bb)->goto_locus);
-- 
2.9.2