Re: [patch] Add discriminators to DWARF line table

[Cary Coutant <ccoutant at google dot com>]

>> ?#ifndef DWARF2_FRAME_INFO
>> @@ -3243,7 +3243,7 @@ dwarf2out_begin_prologue (unsigned int l
>> ? ? ? prologue case, not the eh frame case. ?*/
>> ?#ifdef DWARF2_DEBUGGING_INFO
>> ? ?if (file)
>> - ? ?dwarf2out_source_line (line, file);
>> + ? ?dwarf2out_source_line (line, file, 0);
>
> For a line that uses no discriminators, is 'file:line:0' the same
> as 'file:line'? ?I expect, yes. ?Should we support a
> discriminator other than 0 to be the 'no discriminator' setting?
> I don't know what the standard says, so this may be a silly
> question.

Yes, the default discriminator is 0; I assign non-zero discriminators
whenever there's more than one basic block for that source position. A
discriminator of 0 will not add any new debug info.

>> @@ -91,6 +99,9 @@ static void make_edges (void);
>> ?static void make_cond_expr_edges (basic_block);
>> ?static void make_gimple_switch_edges (basic_block);
>> ?static void make_goto_expr_edges (basic_block);
>> +static unsigned int locus_map_hash (const void*);
>> +static int locus_map_eq (const void*, const void*);
>
> Space before '*'.

OK.

>> +/* Trivial hash function for a location_t. ?*/
>> +
>> +static unsigned int
>> +locus_map_hash (const void* item)
>
> Document ITEM. ?Space before '*'.

OK.

>> +/* Equality function for the locus-to-discriminator map. ?*/
>> +
>> +static int
>> +locus_map_eq (const void* va, const void* vb)
>
> Document VA and VB. ?Space before '*'.

OK.

>> +{
>> + ?const struct locus_discrim_map* a = (const struct locus_discrim_map*) va;
>> + ?const struct locus_discrim_map* b = (const struct locus_discrim_map*) vb;
>
> Likewise.

OK.

>> + ?from = expand_location (locus1);
>> + ?to = expand_location (locus2);
>> + ?return (from.file == to.file && from.line == to.line);
>
> Are file name strings canonicalized by the location_t code? ?I
> assume they are, but I'm not sure. ?If not, you may need a better
> matching code here.

I checked this experimentally, but I'll delve deeper into the code to make sure.

Thanks!

-cary