On Thu, Mar 23, 2017 at 02:13:45PM +0300, Alexander Monakov wrote:
> On Thu, 23 Mar 2017, Jakub Jelinek wrote:
> > On Wed, Mar 22, 2017 at 06:46:34PM +0300, Alexander Monakov wrote:
> > > @@ -4730,6 +4746,25 @@ expand_call_inline (basic_block bb, gimple *stmt,
> > > copy_body_data *id)
> > > if (cfun->gimple_df)
> > > pt_solution_reset (&cfun->gimple_df->escaped);
> > >
> > > + /* Add new automatic variables to IFN_GOMP_SIMT_ENTER arguments. */
> > > + if (id->dst_simt_vars)
> > > + {
> > > + size_t nargs = gimple_call_num_args (simtenter_stmt);
> > > + hash_set<tree> *vars = id->dst_simt_vars;
> > > + auto_vec<tree> newargs (nargs + vars->elements ());
> > > + for (size_t i = 0; i < nargs; i++)
> > > + newargs.quick_push (gimple_call_arg (simtenter_stmt, i));
> > > + for (hash_set<tree>::iterator i = vars->begin (); i != vars->end
> > > (); ++i)
> > > + newargs.quick_push (build1 (ADDR_EXPR,
> > > + build_pointer_type (TREE_TYPE (*i)), *i));
> >
> > Traversing a hash table where the traversal affects code generation is
> > -fcompare-debug unfriendly.
> > Do you actually need a hash_set and not say just a vec of the vars? I can't
> > find where you'd actually do any lookups there, just add and traverse.
>
> Sorry for missing the IR stability issue. This code relies on dst_simt_vars
> being a set and thus having no duplicate entries (so the implicit lookup when
> adding an element is needed).
>
> However, I think I was overly cautious: looking again, I think we can't enter
> copy_decl_for_dup_finish twice with the same 'copy' VAR_DECL. Changing it to
> a
> vec should be fine then?
Yeah, callers of copy_decl* should look the orig var in the decl map first,
plus if you look at all copy_decl_for_dup_finish callers, all of them first
create a new tree (usually copy_node) and then pass it as copy to
copy_decl_for_dup_finish, so you'll never get the same copy in there.
So, please change it into a vector.
Jakub
