PR 27793 - re: C++ duplicate decl uid's

[Andrew MacLeod <amacleod at redhat dot com>]

The patch from PR 26757 which removed an unnecessary hash table guarding
the referenced_var hash table had the side effect of covering up this
issue in the C++ front end for the testcase in that PR.

As the testcase in this PR shows, it doesn't cover them all up, just
some of them :-P

Hopefully there will be a proper fix for 4.2 in the front end, but if I
read Mark's message in 26757 correctly, its unlikely to happen in 4.1.

In any case, as I mentioned in 27793, I can provide a more thorough
workaround now, if we want it. The root problem is that the F/E
generates more than one decl within a function which share the same
DECL_UID.

The machinery for tracking referenced_var in the ssa optimizers assumes
one address per UID.  The table is hashed via UIDs, and contains a
pointer to the decl in the hash slot. 

When ssa is finished, all variables in the hash table get their variable
annotations cleared so when the next function is processed, no decl has
an annotation to start with.

When more than one decl has the same DECL_UID, only one is entered in
the hash table. This means the variable annotations for any other UID
sharing duplicates do *not* get cleared when ssa is deleted.  If one is
fortunate, the global decl's address got entered into the table, and
everything will work fine. If a local decl was entered, then the global
decl will not get its variable annotation cleared, and it will end up
starting the next function with a preassigned (and wrong) ssa_name and
info.  Thats what we're seeing here. 

How to fix it.  If the F/E is going to resolve the issue, then we don't
have to do anything. If it isn't going to be fixed there (for 4.1
perhaps?) then we need to make sure all these annotations are cleared.

The simplest was to do that is to notice when add_referenced_var() is
called that the entry in the hash table doesn't match the variable being
added with the same DECL_UID.  This doesn't happen very often, and the
routine isn't called much, so a linear list ought to be OK.  I've
prepared a patch to do just that, and as you can see, its fairly minor.

In the absence of changing the front end, or reimplementing the way we
clear variable annotations on the way out of SSA, this is probably the
least painful way of resolving the issue.  (At least I hope it resolves
the issue :-)

Opinions? It ain't the prettiest horse in the barn, but it should finish
the race at least. 

Andrew

 


	PR middle-end/27793
	* tree-dfa.c (referenced_vars_dup_list): New.  List of duplicate 
	referenced_variables with matching DECL_UID's.
	(find_referenced_vars): Make sure duplicate list is empty to start.
	(add_referenced_var): Add var to duplicate list if required.
	* tree-ssa.c (delete_tree_ssa): Clear var_ann's on duplicates.
	* tree-flow.h (referenced_vars_dup_list): External declaration.

Index: tree-dfa.c
===================================================================
*** tree-dfa.c	(revision 114129)
--- tree-dfa.c	(working copy)
*************** static void add_referenced_var (tree, bo
*** 76,81 ****
--- 76,83 ----
  
  /* Array of all variables referenced in the function.  */
  htab_t referenced_vars;
+ /* List of referenced variables with duplicate UID's.  */
+ VEC(tree,gc) *referenced_vars_dup_list;
  
  
  /*---------------------------------------------------------------------------
*************** find_referenced_vars (void)
*** 95,100 ****
--- 97,103 ----
    basic_block bb;
    block_stmt_iterator si;
  
+   gcc_assert (VEC_length (tree, referenced_vars_dup_list) == 0);
    FOR_EACH_BB (bb)
      for (si = bsi_start (bb); !bsi_end_p (si); bsi_next (&si))
        {
*************** static void
*** 610,620 ****
  add_referenced_var (tree var, bool always)
  {
    var_ann_t v_ann;
  
    v_ann = get_var_ann (var);
    gcc_assert (DECL_P (var));
    
!   if (always || referenced_var_lookup_if_exists (DECL_UID (var)) == NULL_TREE)
      {
        /* This is the first time we find this variable, add it to the
           REFERENCED_VARS array and annotate it with attributes that are
--- 613,643 ----
  add_referenced_var (tree var, bool always)
  {
    var_ann_t v_ann;
+   tree dup = referenced_var_lookup_if_exists (DECL_UID (var));
  
    v_ann = get_var_ann (var);
    gcc_assert (DECL_P (var));
    
!   /* PRs 26757 and 27793.  Maintain a list of duplicate variable pointers
!      with the same DECL_UID.  There isn't usually very many.  
!      TODO.  Once the C++ front end doesn't create duplicate DECL UID's, this
!      code can be removed.  */
!   if (dup && dup != var)
!     {
!       unsigned u;
!       tree t = NULL_TREE;
! 
!       for (u = 0; u < VEC_length (tree, referenced_vars_dup_list); u++)
! 	{
! 	  t = VEC_index (tree, referenced_vars_dup_list, u);
! 	  if (t == var)
! 	    break;
! 	}
!       if (t != var)
! 	VEC_safe_push (tree, gc, referenced_vars_dup_list, var);
!     }
! 
!   if (always || dup == NULL_TREE)
      {
        /* This is the first time we find this variable, add it to the
           REFERENCED_VARS array and annotate it with attributes that are
Index: tree-ssa.c
===================================================================
*** tree-ssa.c	(revision 114129)
--- tree-ssa.c	(working copy)
*************** delete_tree_ssa (void)
*** 821,826 ****
--- 821,827 ----
    block_stmt_iterator bsi;
    referenced_var_iterator rvi;
    tree var;
+   unsigned u;
  
    /* Release any ssa_names still in use.  */
    for (i = 0; i < num_ssa_names; i++)
*************** delete_tree_ssa (void)
*** 855,860 ****
--- 856,871 ----
        ggc_free (var->common.ann);
        var->common.ann = NULL;
      }
+ 
+   /* Remove any referenced variables which had duplicate UID's.  */
+   for (u = 0; u < VEC_length (tree, referenced_vars_dup_list); u++)
+     {
+       var = VEC_index (tree, referenced_vars_dup_list, u);
+       ggc_free (var->common.ann);
+       var->common.ann = NULL;
+     }
+   VEC_free (tree, gc, referenced_vars_dup_list);
+ 
    htab_delete (referenced_vars);
    referenced_vars = NULL;
  
Index: tree-flow.h
===================================================================
*** tree-flow.h	(revision 114129)
--- tree-flow.h	(working copy)
*************** typedef struct
*** 420,425 ****
--- 420,427 ----
  
  /* Array of all variables referenced in the function.  */
  extern GTY((param_is (struct int_tree_map))) htab_t referenced_vars;
+ /* List of referenced variables in the function with duplicate UID's.  */
+ extern VEC(tree,gc) *referenced_vars_dup_list;
  
  extern tree referenced_var_lookup (unsigned int);
  extern tree referenced_var_lookup_if_exists (unsigned int);