On Wed, Aug 22, 2012 at 1:54 AM, Dehao Chen <de...@google.com> wrote:
> On Tue, Aug 21, 2012 at 6:25 AM, Richard Guenther
> <richard.guent...@gmail.com> wrote:
>> On Mon, Aug 20, 2012 at 3:18 AM, Dehao Chen <de...@google.com> wrote:
>>> ping....
>>
>> Conceptually I like the change. Can a libcpp maintainer please have a 2nd
>> look?
>>
>> Dehao, did you do any compile-time and memory-usage benchmarks?
>
> I don't have a memory benchmarks at hand. But I've tested it through
> some huge apps, each of which takes more than 1 hour to build on a
> modern machine. None of them had observed noticeable memory footprint
> and compile time increase.
Thanks.
I'd like to see a 2nd "ok", but hereby I give a first one ;) It's a big change
but definitely a good one.
Thanks again,
Richard.
> Thanks,
> Dehao
>
>>
>> Thanks,
>> Richard.
>>
>>> Thanks,
>>> Dehao
>>>
>>> On Tue, Aug 14, 2012 at 10:13 AM, Dehao Chen <de...@google.com> wrote:
>>>> Hi, Dodji,
>>>>
>>>> Thanks for the review. I've fixed all the addressed issues. I'm
>>>> attaching the related changes:
>>>>
>>>> Thanks,
>>>> Dehao
>>>>
>>>> libcpp/ChangeLog:
>>>> 2012-08-01 Dehao Chen <de...@google.com>
>>>>
>>>> * include/line-map.h (MAX_SOURCE_LOCATION): New value.
>>>> (location_adhoc_data_init): New.
>>>> (location_adhoc_data_fini): New.
>>>> (get_combined_adhoc_loc): New.
>>>> (get_data_from_adhoc_loc): New.
>>>> (get_location_from_adhoc_loc): New.
>>>> (COMBINE_LOCATION_DATA): New.
>>>> (IS_ADHOC_LOC): New.
>>>> (expanded_location): New field.
>>>> * line-map.c (location_adhoc_data): New.
>>>> (location_adhoc_data_htab): New.
>>>> (curr_adhoc_loc): New.
>>>> (location_adhoc_data): New.
>>>> (allocated_location_adhoc_data): New.
>>>> (location_adhoc_data_hash): New.
>>>> (location_adhoc_data_eq): New.
>>>> (location_adhoc_data_update): New.
>>>> (get_combined_adhoc_loc): New.
>>>> (get_data_from_adhoc_loc): New.
>>>> (get_location_from_adhoc_loc): New.
>>>> (location_adhoc_data_init): New.
>>>> (location_adhoc_data_fini): New.
>>>> (linemap_lookup): Change to use new location.
>>>> (linemap_ordinary_map_lookup): Likewise.
>>>> (linemap_macro_map_lookup): Likewise.
>>>> (linemap_macro_map_loc_to_def_point): Likewise.
>>>> (linemap_macro_map_loc_unwind_toward_spel): Likewise.
>>>> (linemap_get_expansion_line): Likewise.
>>>> (linemap_get_expansion_filename): Likewise.
>>>> (linemap_location_in_system_header_p): Likewise.
>>>> (linemap_location_from_macro_expansion_p): Likewise.
>>>> (linemap_macro_loc_to_spelling_point): Likewise.
>>>> (linemap_macro_loc_to_def_point): Likewise.
>>>> (linemap_macro_loc_to_exp_point): Likewise.
>>>> (linemap_resolve_location): Likewise.
>>>> (linemap_unwind_toward_expansion): Likewise.
>>>> (linemap_unwind_to_first_non_reserved_loc): Likewise.
>>>> (linemap_expand_location): Likewise.
>>>> (linemap_dump_location): Likewise.
>>>>
>>>> Index: libcpp/line-map.c
>>>> ===================================================================
>>>> --- libcpp/line-map.c (revision 190209)
>>>> +++ libcpp/line-map.c (working copy)
>>>> @@ -25,6 +25,7 @@
>>>> #include "line-map.h"
>>>> #include "cpplib.h"
>>>> #include "internal.h"
>>>> +#include "hashtab.h"
>>>>
>>>> static void trace_include (const struct line_maps *, const struct
>>>> line_map *);
>>>> static const struct line_map * linemap_ordinary_map_lookup (struct
>>>> line_maps *,
>>>> @@ -50,6 +51,135 @@
>>>> extern unsigned num_expanded_macros_counter;
>>>> extern unsigned num_macro_tokens_counter;
>>>>
>>>> +/* Data structure to associate an arbitrary data to a source location. */
>>>> +struct location_adhoc_data {
>>>> + source_location locus;
>>>> + void *data;
>>>> +};
>>>> +
>>>> +/* The following data structure encodes a location with some adhoc data
>>>> + and maps it to a new unsigned integer (called an adhoc location)
>>>> + that replaces the original location to represent the mapping.
>>>> +
>>>> + The new adhoc_loc uses the highest bit as the enabling bit, i.e. if the
>>>> + highest bit is 1, then the number is adhoc_loc. Otherwise, it serves as
>>>> + the original location. Once identified as the adhoc_loc, the lower 31
>>>> + bits of the integer is used to index the location_adhoc_data array,
>>>> + in which the locus and associated data is stored. */
>>>> +
>>>> +static htab_t location_adhoc_data_htab;
>>>> +static source_location curr_adhoc_loc;
>>>> +static struct location_adhoc_data *location_adhoc_data;
>>>> +static unsigned int allocated_location_adhoc_data;
>>>> +
>>>> +/* Hash function for location_adhoc_data hashtable. */
>>>> +
>>>> +static hashval_t
>>>> +location_adhoc_data_hash (const void *l)
>>>> +{
>>>> + const struct location_adhoc_data *lb =
>>>> + (const struct location_adhoc_data *) l;
>>>> + return (hashval_t) lb->locus + (size_t) &lb->data;
>>>> +}
>>>> +
>>>> +/* Compare function for location_adhoc_data hashtable. */
>>>> +
>>>> +static int
>>>> +location_adhoc_data_eq (const void *l1, const void *l2)
>>>> +{
>>>> + const struct location_adhoc_data *lb1 =
>>>> + (const struct location_adhoc_data *) l1;
>>>> + const struct location_adhoc_data *lb2 =
>>>> + (const struct location_adhoc_data *) l2;
>>>> + return lb1->locus == lb2->locus && lb1->data == lb2->data;
>>>> +}
>>>> +
>>>> +/* Update the hashtable when location_adhoc_data is reallocated. */
>>>> +
>>>> +static int
>>>> +location_adhoc_data_update (void **slot, void *data)
>>>> +{
>>>> + *((char **) slot) += ((char *) location_adhoc_data - (char *) data);
>>>> + return 1;
>>>> +}
>>>> +
>>>> +/* Combine LOCUS and DATA to a combined adhoc loc. */
>>>> +
>>>> +source_location
>>>> +get_combined_adhoc_loc (source_location locus, void *data)
>>>> +{
>>>> + struct location_adhoc_data lb;
>>>> + struct location_adhoc_data **slot;
>>>> +
>>>> + linemap_assert (data);
>>>> +
>>>> + if (IS_ADHOC_LOC (locus))
>>>> + locus = location_adhoc_data[locus & MAX_SOURCE_LOCATION].locus;
>>>> + if (locus == 0 && data == NULL)
>>>> + return 0;
>>>> + lb.locus = locus;
>>>> + lb.data = data;
>>>> + slot = (struct location_adhoc_data **)
>>>> + htab_find_slot (location_adhoc_data_htab, &lb, INSERT);
>>>> + if (*slot == NULL)
>>>> + {
>>>> + *slot = location_adhoc_data + curr_adhoc_loc;
>>>> + location_adhoc_data[curr_adhoc_loc] = lb;
>>>> + if (++curr_adhoc_loc >= allocated_location_adhoc_data)
>>>> + {
>>>> + char *orig_location_adhoc_data = (char *) location_adhoc_data;
>>>> + allocated_location_adhoc_data *= 2;
>>>> + location_adhoc_data = XRESIZEVEC (struct location_adhoc_data,
>>>> + location_adhoc_data,
>>>> + allocated_location_adhoc_data);
>>>> + htab_traverse (location_adhoc_data_htab,
>>>> location_adhoc_data_update,
>>>> + orig_location_adhoc_data);
>>>> + }
>>>> + }
>>>> + return ((*slot) - location_adhoc_data) | 0x80000000;
>>>> +}
>>>> +
>>>> +/* Return the data for the adhoc loc. */
>>>> +
>>>> +void *
>>>> +get_data_from_adhoc_loc (source_location loc)
>>>> +{
>>>> + linemap_assert (IS_ADHOC_LOC (loc));
>>>> + return location_adhoc_data[loc & MAX_SOURCE_LOCATION].data;
>>>> +}
>>>> +
>>>> +/* Return the location for the adhoc loc. */
>>>> +
>>>> +source_location
>>>> +get_location_from_adhoc_loc (source_location loc)
>>>> +{
>>>> + linemap_assert (IS_ADHOC_LOC (loc));
>>>> + return location_adhoc_data[loc & MAX_SOURCE_LOCATION].locus;
>>>> +}
>>>> +
>>>> +/* Initialize the location_adhoc_data structure. */
>>>> +
>>>> +void
>>>> +location_adhoc_data_init (void)
>>>> +{
>>>> + location_adhoc_data_htab = htab_create (100, location_adhoc_data_hash,
>>>> + location_adhoc_data_eq, NULL);
>>>> + curr_adhoc_loc = 0;
>>>> + allocated_location_adhoc_data = 100;
>>>> + location_adhoc_data = XNEWVEC (struct location_adhoc_data,
>>>> + allocated_location_adhoc_data);
>>>> +}
>>>> +
>>>> +/* Finalize the location_adhoc_data structure. */
>>>> +
>>>> +void
>>>> +location_adhoc_data_fini (void)
>>>> +{
>>>> + allocated_location_adhoc_data = 0;
>>>> + XDELETEVEC (location_adhoc_data);
>>>> + htab_delete (location_adhoc_data_htab);
>>>> +}
>>>> +
>>>> /* Initialize a line map set. */
>>>>
>>>> void
>>>> @@ -509,6 +639,8 @@
>>>> const struct line_map*
>>>> linemap_lookup (struct line_maps *set, source_location line)
>>>> {
>>>> + if (IS_ADHOC_LOC (line))
>>>> + line = location_adhoc_data[line & MAX_SOURCE_LOCATION].locus;
>>>> if (linemap_location_from_macro_expansion_p (set, line))
>>>> return linemap_macro_map_lookup (set, line);
>>>> return linemap_ordinary_map_lookup (set, line);
>>>> @@ -525,6 +657,9 @@
>>>> unsigned int md, mn, mx;
>>>> const struct line_map *cached, *result;
>>>>
>>>> + if (IS_ADHOC_LOC (line))
>>>> + line = location_adhoc_data[line & MAX_SOURCE_LOCATION].locus;
>>>> +
>>>> if (set == NULL || line < RESERVED_LOCATION_COUNT)
>>>> return NULL;
>>>>
>>>> @@ -570,6 +705,9 @@
>>>> unsigned int md, mn, mx;
>>>> const struct line_map *cached, *result;
>>>>
>>>> + if (IS_ADHOC_LOC (line))
>>>> + line = location_adhoc_data[line & MAX_SOURCE_LOCATION].locus;
>>>> +
>>>> linemap_assert (line >= LINEMAPS_MACRO_LOWEST_LOCATION (set));
>>>>
>>>> if (set == NULL)
>>>> @@ -648,6 +786,9 @@
>>>> {
>>>> unsigned token_no;
>>>>
>>>> + if (IS_ADHOC_LOC (location))
>>>> + location = location_adhoc_data[location & MAX_SOURCE_LOCATION].locus;
>>>> +
>>>> linemap_assert (linemap_macro_expansion_map_p (map)
>>>> && location >= MAP_START_LOCATION (map));
>>>> linemap_assert (location >= RESERVED_LOCATION_COUNT);
>>>> @@ -672,6 +813,9 @@
>>>> {
>>>> unsigned token_no;
>>>>
>>>> + if (IS_ADHOC_LOC (location))
>>>> + location = location_adhoc_data[location & MAX_SOURCE_LOCATION].locus;
>>>> +
>>>> linemap_assert (linemap_macro_expansion_map_p (map)
>>>> && location >= MAP_START_LOCATION (map));
>>>> linemap_assert (location >= RESERVED_LOCATION_COUNT);
>>>> @@ -696,6 +840,9 @@
>>>> {
>>>> const struct line_map *map = NULL;
>>>>
>>>> + if (IS_ADHOC_LOC (location))
>>>> + location = location_adhoc_data[location & MAX_SOURCE_LOCATION].locus;
>>>> +
>>>> if (location < RESERVED_LOCATION_COUNT)
>>>> return 0;
>>>>
>>>> @@ -720,6 +867,9 @@
>>>> {
>>>> const struct line_map *map = NULL;
>>>>
>>>> + if (IS_ADHOC_LOC (location))
>>>> + location = location_adhoc_data[location & MAX_SOURCE_LOCATION].locus;
>>>> +
>>>> if (location < RESERVED_LOCATION_COUNT)
>>>> return NULL;
>>>>
>>>> @@ -754,6 +904,9 @@
>>>> {
>>>> const struct line_map *map = NULL;
>>>>
>>>> + if (IS_ADHOC_LOC (location))
>>>> + location = location_adhoc_data[location & MAX_SOURCE_LOCATION].locus;
>>>> +
>>>> if (location < RESERVED_LOCATION_COUNT)
>>>> return false;
>>>>
>>>> @@ -793,6 +946,9 @@
>>>> linemap_location_from_macro_expansion_p (struct line_maps *set,
>>>> source_location location)
>>>> {
>>>> + if (IS_ADHOC_LOC (location))
>>>> + location = location_adhoc_data[location & MAX_SOURCE_LOCATION].locus;
>>>> +
>>>> linemap_assert (location <= MAX_SOURCE_LOCATION
>>>> && (set->highest_location
>>>> < LINEMAPS_MACRO_LOWEST_LOCATION (set)));
>>>> @@ -933,6 +1089,9 @@
>>>> {
>>>> struct line_map *map;
>>>>
>>>> + if (IS_ADHOC_LOC (location))
>>>> + location = location_adhoc_data[location & MAX_SOURCE_LOCATION].locus;
>>>> +
>>>> linemap_assert (set && location >= RESERVED_LOCATION_COUNT);
>>>>
>>>> while (true)
>>>> @@ -967,6 +1126,9 @@
>>>> {
>>>> struct line_map *map;
>>>>
>>>> + if (IS_ADHOC_LOC (location))
>>>> + location = location_adhoc_data[location & MAX_SOURCE_LOCATION].locus;
>>>> +
>>>> linemap_assert (set && location >= RESERVED_LOCATION_COUNT);
>>>>
>>>> while (true)
>>>> @@ -1005,6 +1167,9 @@
>>>> {
>>>> struct line_map *map;
>>>>
>>>> + if (IS_ADHOC_LOC (location))
>>>> + location = location_adhoc_data[location & MAX_SOURCE_LOCATION].locus;
>>>> +
>>>> linemap_assert (set && location >= RESERVED_LOCATION_COUNT);
>>>>
>>>> while (true)
>>>> @@ -1074,6 +1239,9 @@
>>>> enum location_resolution_kind lrk,
>>>> const struct line_map **map)
>>>> {
>>>> + if (IS_ADHOC_LOC (loc))
>>>> + loc = location_adhoc_data[loc & MAX_SOURCE_LOCATION].locus;
>>>> +
>>>> if (loc < RESERVED_LOCATION_COUNT)
>>>> {
>>>> /* A reserved location wasn't encoded in a map. Let's return a
>>>> @@ -1121,6 +1289,9 @@
>>>> source_location resolved_location;
>>>> const struct line_map *resolved_map;
>>>>
>>>> + if (IS_ADHOC_LOC (loc))
>>>> + loc = location_adhoc_data[loc & MAX_SOURCE_LOCATION].locus;
>>>> +
>>>> resolved_location =
>>>> linemap_macro_map_loc_unwind_toward_spelling (*map, loc);
>>>> resolved_map = linemap_lookup (set, resolved_location);
>>>> @@ -1157,6 +1328,9 @@
>>>> source_location resolved_loc;
>>>> const struct line_map *map0 = NULL, *map1 = NULL;
>>>>
>>>> + if (IS_ADHOC_LOC (loc))
>>>> + loc = location_adhoc_data[loc & MAX_SOURCE_LOCATION].locus;
>>>> +
>>>> map0 = linemap_lookup (set, loc);
>>>> if (!linemap_macro_expansion_map_p (map0))
>>>> return loc;
>>>> @@ -1198,6 +1372,11 @@
>>>> expanded_location xloc;
>>>>
>>>> memset (&xloc, 0, sizeof (xloc));
>>>> + if (IS_ADHOC_LOC (loc))
>>>> + {
>>>> + loc = location_adhoc_data[loc & MAX_SOURCE_LOCATION].locus;
>>>> + xloc.data = location_adhoc_data[loc & MAX_SOURCE_LOCATION].data;
>>>> + }
>>>>
>>>> if (loc < RESERVED_LOCATION_COUNT)
>>>> /* The location for this token wasn't generated from a line map.
>>>> @@ -1290,6 +1469,9 @@
>>>> const char *path = "", *from = "";
>>>> int l = -1, c = -1, s = -1, e = -1;
>>>>
>>>> + if (IS_ADHOC_LOC (loc))
>>>> + loc = location_adhoc_data[loc & MAX_SOURCE_LOCATION].locus;
>>>> +
>>>> if (loc == 0)
>>>> return;
