Re: [PATCH] Fix lost locus issues at -O0 -g (PRs debug/29609, debug/36690, debug/37616)

[Ian Lance Taylor <iant at google dot com>]

Jakub Jelinek <jakub@redhat.com> writes:

> But this leads to questions about the internal representation of the
> RTL locators.  Currently we have 2 pairs of vectors, one used separately for
> location_t and one for block, always one vector in the pair has int values
> and the other has location_t (also 32-bit) or tree (32-bit or 64-bit
> depending on host) and the lookup looks quite expensive (binary search).
> For 32-bit hosts, wouldn't just using 2 vectors, one with location_t and one
> with tree, both indexed by the RTL locator int, be always more compact (and
> obviously faster)?  For 64-bit hosts perhaps this could be slightly bigger
> than current representation if there are far more location_t changes than
> block changes (likely), but at least couldn't we avoid the binary
> search for the location_t side and just use directly indexed vector?

Yes, the current approach has been clearly broken since we went to
mapped locations.  I think the ideal approach would be to encode the
binding-block number into the location_t, and have a vector mapping
those numbers to binding blocks.  That would give us relatively
efficient access to all the information we need for minimal storage.
We could then proceed to eliminate the block field in
gimple_statement_base and the block field in tree_exp.  The downside
would be that on very large files we could run out of bits in
location_t, so we might need to have an escape encoding.  The escape
encoding would cause us to look in a pointer_map from
trees/gimples/RTL to a binding block.  That would require some
attention any time we copied a location.  Fortunately the logic could
be encapsulated in a single shared function, and there aren't that
many plases where we copy a location.

Also, the approach of using reemit_insn_block_notes seems nuts in
today's compiler; block notes are basically only used by
final_scan_insn, which could certainly do the same thing as
reemit_insn_block_notes without using the extra memory.


> 2008-10-08  Jakub Jelinek  <jakub@redhat.com>
>
> 	* cfgexpand.c (expand_gimple_cond): Convert also goto_block and
> 	goto_locus of true_edge into RTL locator.

This patch is OK.

Thanks.

Ian