[committed, gomp4] Fix release_dangling_ssa_names

[Richard Biener]

On Wed, 5 Aug 2015, Tom de Vries wrote:

> [ was: Re: Expand oacc kernels after pass_fre ]
> On 04/06/15 18:02, Tom de Vries wrote:
> > > Please move this out of the class body.
> > > 
> > 
> > Fixed and committed (ommitting patch as trivial).
> > 
> > > > +    {
> > > > +      unsigned res = execute_expand_omp ();
> > > > +
> > > > +      /* After running pass_expand_omp_ssa to expand the oacc kernels
> > > > +     directive, we are left in the original function with anonymous
> > > > +     SSA_NAMEs, with a defining statement that has been deleted.  This
> > > > +     pass finds those SSA_NAMEs and releases them.
> > > > +     TODO: Either fix this elsewhere, or make the fix unnecessary.  */
> > > > +      unsigned int i;
> > > > +      for (i = 1; i < num_ssa_names; ++i)
> > > > +    {
> > > > +      tree name = ssa_name (i);
> > > > +      if (name == NULL_TREE)
> > > > +        continue;
> > > > +
> > > > +      gimple stmt = SSA_NAME_DEF_STMT (name);
> > > > +      bool found = false;
> > > > +
> > > > +      ssa_op_iter op_iter;
> > > > +      def_operand_p def_p;
> > > > +      FOR_EACH_PHI_OR_STMT_DEF (def_p, stmt, op_iter, SSA_OP_ALL_DEFS)
> > > > +        {
> > > > +          tree def = DEF_FROM_PTR (def_p);
> > > > +          if (def == name)
> > > > +        {
> > > > +          found = true;
> > > > +          break;
> > > > +        }
> > > > +        }
> > > > +
> > > > +      if (!found)
> > > > +        {
> > > > +          if (dump_file)
> > > > +        fprintf (dump_file, "Released dangling ssa name %u\n", i);
> > > > +          release_ssa_name (name);
> > > > +        }
> > > > +    }
> > > > +
> > > > +      return res;
> > > > +    }
> 
> This patch implements the TODO.
> 
> The cause of the problems is that in replace_ssa_name, we create a new
> ssa_name with the def stmt of the old ssa_name, but do not reset the def stmt
> of the old ssa_name, leaving the ssa_name in the original function having a
> def stmt in the split-off function.
> 
> [ And if we don't do anything about that, at some point in another pass we use
> 'gimple_bb (SSA_NAME_DEF_STMT (name))->index' (a bb index in the split-off
> function) as an index into an array with as length the number of bbs in the
> original function. So the index may be out of bounds. ]
> 
> This patch fixes that by making sure we reset the def stmt to NULL. This means
> we can simplify release_dangling_ssa_names to just test for NULL def stmts.

Not sure if I understand the problem correctly but why are you not simply
releasing the SSA name when you remove its definition?

Richard.

> Default defs are skipped by release_ssa_name, so setting the def stmt for
> default defs to NULL does not result in the name being released, but in an
> ssa-verification error. So instead, we keep the def stmt nop, and create a new
> nop for the copy in the split-off function.
> 
> [ The default def bit seems only to be triggered for the default def created
> by expand_omp_target:
> ...
>   /* If we are in ssa form, we must load the value from the default
>      definition of the argument.  That should not be defined now,
>      since the argument is not used uninitialized.  */
>   gcc_assert (ssa_default_def (cfun, arg) == NULL);
>   narg = make_ssa_name (arg, gimple_build_nop ());
>   set_ssa_default_def (cfun, arg, narg);
> ...
> ]
> 
> Bootstrapped and reg-tested on x86_64.
> 
> Committed to gomp-4_0-branch.
> 
> Thanks,
> - Tom
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham Norton, HRB 21284 (AG Nuernberg)