This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Help with rich_location and GIMPLE stmts


On Mon, 2017-05-15 at 15:36 +0200, Martin Liška wrote:
> Hi.
> 
> I sent this email to David some time ago, but it should be probably
> answered
> on gcc mailing list.

> I have idea one to improve gcov tool and I'm interested in more
> precise locations for gimple
> statements. For gcov purpose, we dump location in ipa-profile pass,
> which is an early IPA
> pass and this data is used by gcov tool to map statements (blocks) to
> lines of code.
> 
> I did a small experiment on the place we emit the location data:
> 		  inform (gimple_location (stmt), "output_location");
> 
> and it shows for:
> $ cat m2.c
> unsigned int
> UuT (void)
> { unsigned int true_var = 1; unsigned int false_var = 0; unsigned int
> ret = 0; if (true_var) /* count(1) */ { if (false_var) /* count(1) */
> ret = 111; /* count(#####) */ } else ret = 999; /* count(#####) */
> return ret; }
> 
> int
> main (int argc, char **argv)
> {
>   UuT ();
>   return 0;
> }
> 
> $ gcc --coverage m2.c
> m2.c: In function ‘main’:
> m2.c:8:3: note: output_location
>    UuT ();
>    ^~~~~~
> # .MEM_2 = VDEF <.MEM_1(D)>
> UuT ();
> m2.c:9:10: note: output_location
>    return 0;
>           ^
> _3 = 0;
> m2.c: In function ‘UuT’:
> m2.c:3:16: note: output_location
>  { unsigned int true_var = 1; unsigned int false_var = 0; unsigned
> int ret = 0; if (true_var) /* count(1) */ { if (false_var) /*
> count(1) */ ret = 111; /* count(#####) */ } else ret = 999; /*
> count(#####) */ return ret; }
>                 ^~~~~~~~
> true_var_3 = 1;
> m2.c:3:43: note: output_location
>  { unsigned int true_var = 1; unsigned int false_var = 0; unsigned
> int ret = 0; if (true_var) /* count(1) */ { if (false_var) /*
> count(1) */ ret = 111; /* count(#####) */ } else ret = 999; /*
> count(#####) */ return ret; }
>                                            ^~~~~~~~~
> false_var_4 = 0;
> m2.c:3:71: note: output_location
>  { unsigned int true_var = 1; unsigned int false_var = 0; unsigned
> int ret = 0; if (true_var) /* count(1) */ { if (false_var) /*
> count(1) */ ret = 111; /* count(#####) */ } else ret = 999; /*
> count(#####) */ return ret; }
>                                                                      
>   ^~~
> ret_5 = 0;
> m2.c:3:83: note: output_location
>  { unsigned int true_var = 1; unsigned int false_var = 0; unsigned
> int ret = 0; if (true_var) /* count(1) */ { if (false_var) /*
> count(1) */ ret = 111; /* count(#####) */ } else ret = 999; /*
> count(#####) */ return ret; }
>                                                                      
>               ^
> if (true_var_3 != 0)
> m2.c:3:114: note: output_location
>  { unsigned int true_var = 1; unsigned int false_var = 0; unsigned
> int ret = 0; if (true_var) /* count(1) */ { if (false_var) /*
> count(1) */ ret = 111; /* count(#####) */ } else ret = 999; /*
> count(#####) */ return ret; }
>                                                                      
>                                              ^
> if (false_var_4 != 0)
> m2.c:3:145: note: output_location
>  { unsigned int true_var = 1; unsigned int false_var = 0; unsigned
> int ret = 0; if (true_var) /* count(1) */ { if (false_var) /*
> count(1) */ ret = 111; /* count(#####) */ } else ret = 999; /*
> count(#####) */ return ret; }
>                                                                      
>                                                                      
>    ~~~~^~~~~
> ret_7 = 111;
> m2.c:3:182: note: output_location
>  { unsigned int true_var = 1; unsigned int false_var = 0; unsigned
> int ret = 0; if (true_var) /* count(1) */ { if (false_var) /*
> count(1) */ ret = 111; /* count(#####) */ } else ret = 999; /*
> count(#####) */ return ret; }
>                                                                      
>                                                                      
>                                         ~~~~^~~~~
> ret_6 = 999;
> m2.c:3:215: note: output_location
>  { unsigned int true_var = 1; unsigned int false_var = 0; unsigned
> int ret = 0; if (true_var) /* count(1) */ { if (false_var) /*
> count(1) */ ret = 111; /* count(#####) */ } else ret = 999; /*
> count(#####) */ return ret; }
>                                                                      
>                                                                      
>                                                                      
>         ^~~
> _8 = ret_2;
> m2.c:3:215: note: output_location
> # VUSE <.MEM_9(D)>
> return _8;
> 
> Which is not optimal, for some assignments I see just LHS 
> (false_var_4 = 0),

My first though was: are there assignments for which this isn't the
case?  The only one I see is the:
  ret = 999;
  ~~~~^~~~~

Are the locations for these assignments coming through from the
frontend?

I believe that in the C frontend these are assignment-expression, and
hence handled by c_parser_expr_no_commas; in particular the use of
op_location and the call:

  set_c_expr_source_range (&ret, lhs.get_start (), rhs.get_finish ());

ought to be setting up the caret of the assignment to be on the
operator token, and for the start/finish to range from the start of the
lhs to the end of the rhs i.e. what we see for:

  ret = 999;
  ~~~~^~~~~


> for return statements only a returned value is displayed. 

Is this running on SSA form?  If so, I wonder if you're running into
something like this:

  retval_N = PHI <lots of values>;
  return retval_N;

where it's using the location of that "return retval_N;" for all of the
return statements in the function, rather than the individual source
locations.

> For conditions, only condition beginning is showed.
> Is this known behavior or do I miss
> something?

c_parser_if_statement has:

  loc = c_parser_peek_token (parser)->location;

which is that of the open-paren.  Maybe we should build a location
covering the range of the "if ( expression )" part of the if-statement?

Note that the C++ frontend may do different things than the C frontend
for any and all of these.  When I added range-based locations to them,
I attempted to preserve the caret locations to avoid affecting stepping
behavior in the debugger (since the caret location is used when
stripping away range information); we could revisit some of the
decisions.

BTW, in the subject line, you mention a "rich_location", but I think
you mean a source_location (aka location_t): this is a caret location
along with range information; a rich_location is one or more of them,
potentially with fix-it hints.  See "Example B" and "Example C" in line
-map.h.  Admittedly the terminology there is a little muddled (sorry).


Dave


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]