On Mon, 30 Nov 2015, Tom de Vries wrote:
> On 30/11/15 10:16, Richard Biener wrote:
> > On Mon, 30 Nov 2015, Tom de Vries wrote:
> >
> > > Hi,
> > >
> > > this patch fixes PR46032.
> > >
> > > It handles a call:
> > > ...
> > > __builtin_GOMP_parallel (fn, data, num_threads, flags)
> > > ...
> > > as:
> > > ...
> > > fn (data)
> > > ...
> > > in ipa-pta.
> > >
> > > This improves ipa-pta alias analysis in the parallelized function fn, and
> > > allows vectorization in the testcase without a runtime alias test.
> > >
> > > Bootstrapped and reg-tested on x86_64.
> > >
> > > OK for stage3 trunk?
> >
> > + /* Assign the passed argument to the appropriate incoming
> > + parameter of the function. */
> > + struct constraint_expr lhs ;
> > + lhs = get_function_part_constraint (fi, fi_parm_base + 0);
> > + auto_vec<ce_s, 2> rhsc;
> > + struct constraint_expr *rhsp;
> > + get_constraint_for_rhs (arg, &rhsc);
> > + while (rhsc.length () != 0)
> > + {
> > + rhsp = &rhsc.last ();
> > + process_constraint (new_constraint (lhs, *rhsp));
> > + rhsc.pop ();
> > + }
> >
> > please use style used elsewhere with
> >
> > FOR_EACH_VEC_ELT (rhsc, j, rhsp)
> > process_constraint (new_constraint (lhs, *rhsp));
> > rhsc.truncate (0);
> >
>
> That code was copied from find_func_aliases_for_call.
> I've factored out the bit that I copied as find_func_aliases_for_call_arg, and
> fixed the style there (and dropped 'rhsc.truncate (0)' since AFAIU it's
> redundant at the end of a function).
>
> > + /* Parameter passed by value is used. */
> > + lhs = get_function_part_constraint (fi, fi_uses);
> > + struct constraint_expr *rhsp;
> > + get_constraint_for_address_of (arg, &rhsc);
> >
> > This isn't correct - you want to use get_constraint_for (arg, &rhsc).
> > After all rhs is already an ADDR_EXPR.
> >
>
> Can we add an assert somewhere to detect this incorrect usage?
>
> > + FOR_EACH_VEC_ELT (rhsc, j, rhsp)
> > + process_constraint (new_constraint (lhs, *rhsp));
> > + rhsc.truncate (0);
> > +
> > + /* The caller clobbers what the callee does. */
> > + lhs = get_function_part_constraint (fi, fi_clobbers);
> > + rhs = get_function_part_constraint (cfi, fi_clobbers);
> > + process_constraint (new_constraint (lhs, rhs));
> > +
> > + /* The caller uses what the callee does. */
> > + lhs = get_function_part_constraint (fi, fi_uses);
> > + rhs = get_function_part_constraint (cfi, fi_uses);
> > + process_constraint (new_constraint (lhs, rhs));
> >
> > I don't see why you need those. The solver should compute these
> > in even better precision (context sensitive on the call side).
> >
> > The same is true for the function parameter. That is, the only
> > needed part of the patch should be that making sure we see
> > the "direct" call and assign parameters correctly.
> >
>
> Dropped this bit.
>
> OK for stage3 trunk if bootstrap and reg-test succeeds?
- || node->address_taken);
+ || (node->address_taken
+ && !node->parallelized_function));
please add a comment here on why this is safe.
Ok with this change.
Thanks,
Richard.
