On Mon, Jan 26, 2015 at 10:11:14AM +0100, Richard Biener wrote:
> On Sat, Jan 24, 2015 at 12:23 AM, Alan Modra <amo...@gmail.com> wrote:
> > How does this look as a potential fix for PR64703? I haven't made
> > many forays into gimple code, so even though this patch passes
> > bootstrap and regression testing on powerpc64-linux it's quite
> > possible this is the wrong place to change. If it does look to be OK,
> > then I'll fill out the targetm changes, include a testcase etc.
>
> It looks mostly ok, comments below.
Thanks for looking!
> > PR target/64703
> > * tree-ssa-alias.c (pt_solution_includes_base): New function,
> > extracted from..
> > (ref_maybe_used_by_call_p_1): ..here. Delete dead code checking
> > for NULL return from ao_ref_base. Handle potential memory
> > reference by indirect calls on targets using function descriptors.
> >
> > Index: gcc/tree-ssa-alias.c
> > ===================================================================
> > --- gcc/tree-ssa-alias.c (revision 220025)
> > +++ gcc/tree-ssa-alias.c (working copy)
> > @@ -1532,6 +1532,23 @@ refs_output_dependent_p (tree store1, tree store2)
> > return refs_may_alias_p_1 (&r1, &r2, false);
> > }
> >
> > +static bool
> > +pt_solution_includes_base (struct pt_solution *pt, tree base)
>
> Needs a comment.
Sure, that was part of "etc." above. ;)
> > +{
> > + if (DECL_P (base))
> > + return pt_solution_includes (pt, base);
> > +
> > + if ((TREE_CODE (base) == MEM_REF
> > + || TREE_CODE (base) == TARGET_MEM_REF)
> > + && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME)
> > + {
> > + struct ptr_info_def *pi = SSA_NAME_PTR_INFO (TREE_OPERAND (base, 0));
> > + if (pi)
> > + return pt_solutions_intersect (pt, &pi->pt);
> > + }
> > + return true;
> > +}
> > +
> > /* If the call CALL may use the memory reference REF return true,
> > otherwise return false. */
> >
> > @@ -1542,15 +1559,24 @@ ref_maybe_used_by_call_p_1 (gcall *call, ao_ref *r
> > unsigned i;
> > int flags = gimple_call_flags (call);
> >
> > + base = ao_ref_base (ref);
>
> You dropped the
>
> if (!base)
> return true;
>
> check - please put it back.
Hmm, calls to ao_ref_base in tree-ssa-alias.c are a mixed bag. Some
check the return for NULL, others don't. All the checks for NULL are
dead code since ao_ref_base never returns NULL. OK, I'll put it back
and leave cleanup for another day.
>
> > + callee = gimple_call_fn (call);
> > + if (callee && TREE_CODE (callee) == SSA_NAME
>
> Do we never propagate the address of a function descriptor here?
> That is, can we generate a testcase like
>
> descr fn;
> <setup fn>
> foo (&fn);
>
> void foo (fn)
> {
> (*fn) (a, b);
> }
>
> and then inline foo so the call becomes
>
> (*&fn) (a, b);
>
> ? We'd then still implicitely read from 'fn'. I also wonder whether
> (and where!) we resolve such a descriptor reference to the actual
> function on GIMPLE?
Well, if we did end up with a direct call then the value in 'fn' won't
matter, I think. A direct call implies gcc knows the value of a
function pointer, and can thus use the value rather than the pointer.
In this case it implies gcc has looked through 'fn' to its
initialization, and seen that 'fn' is a function address.
ie. your <setup fn> above is something like
fn = *(descr *) &some_function;
Now it appears that gcc isn't clever enough to do this, but if it did,
then surely the "value of the pointer" would be 'some_function', not
'fn'.
I can't see how we could end up with a direct call any other way. As
far as I know, gcc doesn't have any knowledge that 'descr' has
anything to do with a function address unless that knowledge is
imparted by an initialization such as the above. ie. The answer to
your "and where!" question is "nowhere".
> That is - don't you simply want to use
>
> if (targetm.function_descriptors
> && ptr_deref_mayalias_ref_p_1 (callee, ref))
> return true;
>
> here?
No. I don't want to do needless work on direct calls, particularly
since it appears that ptr_deref_may_alias_ref_p_1 does return true for
direct calls like memcpy.
--
Alan Modra
Australia Development Lab, IBM
