Re: int_cst_hash_table mapping persistence and the garbage collector

[Gary Funck <gary@intrepid.com>]

On 10/11/11 11:05:18, Eric Botcazou wrote:
> > One easy way to address the current issue is to call
> > tree_int_cst_equal() if the integer constant tree pointers
> > do not match:
> >
> >     if ((c1 != c2) && !tree_int_cst_equal (c1, c2))
> >       /* integer constants aren't equal.  */
> 
> You have two objects C1 and C2 for the same constant and you're comparing
> them. One was created first, say C1.  If C1 was still live when C2 was
> created, why was C2 created in the first class?  If C1 wasn't live
> anymore when C2 was created, why are you still using C1 here?

Eric, this note provides some more detail in addition to my
earlier reply to Richard.

The problem is that the references to object C1 and C2 live in
a hash table, and that although the referenced nodes will be retained
by the garbage collector, their mapping in int_cst_hash_table is
deleted by the GC.

Thus, if we follow the diagram:

 tree (type) -> [ upc_block_factor_for_type ]
                 -> tree (integer constant)
 tree (integer constant) -> [ int_cst_hash_table ] {unique map}
                         -> tree (integer constant)

Given two tree nodes, P (prototype) and F (function) they declare
a parameter that is pointer to a UPC shared object and this pointer
is declared with a UPC blocking factor of 1000.  Without garbage
collection, the mappings look like this:

 P.param -> C1, F.param -> C1 where C1 is an integer constant of
                           the form (sizetype, 1000).

but when GC kicks in, it decides that the hash table entry
in int_cst_hash_table can be deleted because it doesn't think
that C1 is live.  Therefore the next attempt to map (sizetype, 1000)
will yield a new integral constant tree node, C2.  Then the mapping
changes to: P.param -> C1, F.param -> C2, and we can no longer use

  TYPE_UPC_BLOCKING_FACTOR (P.param) == TYPE_UPC_BLOCKING_FACTOR (F.param)

to check that the blocking factors of P.param and F.param are equal.

For the GC to know that int_cst_hash_table
entry is needed, perhaps the upc_block_factor_for_type needs
to be traversed, and then each integer constant needs to be
"marked", or the constant has to be hashed into int_cst_hash_table
and the actual hash table entry has to be marked.

I am not familiar with the details of garbage collection and
pretty much just try use existing code as a model.  Apparently,
this sequence of statements is insufficient to tell the GC that it should
mark the integer constants referenced in this hash table as "in use".

  static GTY ((if_marked ("tree_map_marked_p"),
	     param_is (struct tree_map)))
       htab_t upc_block_factor_for_type;
[...]
 upc_block_factor_for_type = htab_create_ggc (512, tree_map_hash
                                              tree_map_eq, 0);

Reading the GC code:

static int
ggc_htab_delete (void **slot, void *info)
{
  const struct ggc_cache_tab *r = (const struct ggc_cache_tab *) info;

  if (! (*r->marked_p) (*slot))
    htab_clear_slot (*r->base, slot);
  else
    (*r->cb) (*slot);

  return 1;
}

It appears that the int_cst_hash_table entry for C1 needs to be
marked or it will be cleared.  I don't know how to set things
up so that so that the garbage collecting mechanisms are in
place to do that, and was hoping that tree_map_hash table
would provide the required mechanisms.  Apparently, this is
not the case.

I had hoped that this declaration would be sufficient to convince
the GC to consider all mapped integer constant nodes to be "live".
If not, then perhaps I need a GC hook associated with
upc_block_factor_for_type that does something like the following:

  for t (each used upc_block_factor_for_type entry):
    c = t.to  # the mapped integer constant
    if is_integer_constant (c):
      h = int_cst_hash_table.hash(c)
      gc_mark_htab (int_cst_hash_table, h)

or perhaps this is sufficient?

  for t (each used upc_block_factor_for_type entry):
    c = t.to
    gc_mark_tree_node (c)

However, I thought that this would already have been done
automatically by the GC hash tree implementation.

If either of those methods are required, I would appreciate
suggestions/pointers/code that would help me make sure that
this approach is implemented correctly.

thanks,
- Gary