On Thu, Sep 24, 2020 at 12:54 PM Jan Hubicka <hubi...@ucw.cz> wrote:
>
> > > + else if (TREE_CODE (op) == SSA_NAME
> > > + && POINTER_TYPE_P (TREE_TYPE (op)))
> > > + {
> > > + if (DECL_P (base) && !ptr_deref_may_alias_decl_p (op, base))
> > > + return false;
> > > + if (TREE_CODE (base) == SSA_NAME
> > > + && !ptr_derefs_may_alias_p (op, base))
> > > + return false;
> > > + }
> >
> > this all looks redundant - why is it important to look at the
> > base of ref, why not simply ask below (*)
> >
> > > + modref_access_node *access_node;
> > > + FOR_EACH_VEC_SAFE_ELT (ref_node->accesses, k, access_node)
> > > + {
> > > + if (num_tests >= max_tests)
> > > + return true;
> > > +
> > > + if (access_node->parm_index == -1
> > > + || (unsigned)access_node->parm_index
> > > + >= gimple_call_num_args (stmt))
> > > + return true;
> > > +
> > > + tree op = gimple_call_arg (stmt, access_node->parm_index);
> > > +
> > > + alias_stats.modref_baseptr_tests++;
> > > +
> > > + /* Lookup base, if this is the first time we compare bases.
> > > */
> > > + if (!base)
> >
> > Meh, so this function is a bit confusing with base_node, ref_node,
> > access_node and now 'base' and 'op'. The loop now got a
> > new nest as well.
> >
> > I'm looking for a high-level description of the modref_tree <>
> > but cannot find any which makes reviewing this quite difficult...
>
> There is a description in ipa-modref.c though it may need a bit of expanding.
> Basically the modref summary represents a decision tree for
> tree-ssa-alias that has three levels
> 1) base which records base alias set,
> 2) ref which records ref alias set and
> 3) access wich presently records info whether the access is a
> dreference of pointer passed by parameter. In future I will re-add
> info about offset/size and base type. It would be posisble to record
> the access path though I am not sure if that it is worth the effort
> >
> > > + {
> > > + base = ref->ref;
> > > + while (handled_component_p (base))
> > > + base = TREE_OPERAND (base, 0);
> >
> > ao_ref_base (ref)? OK, that might strip an inner
> > MEM_REF, yielding in a decl, but ...
> >
> > > + if (TREE_CODE (base) == MEM_REF
> > > + || TREE_CODE (base) == TARGET_MEM_REF)
> > > + base = TREE_OPERAND (base, 0);
> >
> > that might happen here, too. But in the MEM_REF case base
> > is a pointer.
> >
> > > + }
> > > +
> > > + if (base_may_alias_with_dereference_p (base, op))
> >
> > So this is a query purely at the caller side - whether 'ref' may
> > alias 'op'.
> >
> > ---
> >
> > (*) ptr_deref_may_alias_ref_p_1 (op, ref)
> >
> > without any of the magic?
>
> Hmm, it may actually just work, I did not know that looks through
> memrefs, let me re-test the patch.
> >
> > Can you please amend ipa-modref-tree.h/c with a toplevel comment
> > layint out the data structure and what is recorded?
>
> I will do (but need to think bit of a redundancy between comment in
> ipa-modref and ipa-modref-tree)
One place is enough - just add a pointer to the other place.
Richard.
> Honza
> >
> > Thanks,
> > Richard.
> >
> > > + return true;
> > > + num_tests++;
> > > + }
> > > }
> > > }
> > > return false;
> > > @@ -2510,7 +2584,7 @@ ref_maybe_used_by_call_p_1 (gcall *call, ao_ref
> > > *ref, bool tbaa_p)
> > > modref_summary *summary = get_modref_function_summary (node);
> > > if (summary)
> > > {
> > > - if (!modref_may_conflict (summary->loads, ref, tbaa_p))
> > > + if (!modref_may_conflict (call, summary->loads, ref,
> > > tbaa_p))
> > > {
> > > alias_stats.modref_use_no_alias++;
> > > if (dump_file && (dump_flags & TDF_DETAILS))
> > > @@ -2934,7 +3008,7 @@ call_may_clobber_ref_p_1 (gcall *call, ao_ref *ref,
> > > bool tbaa_p)
> > > modref_summary *summary = get_modref_function_summary (node);
> > > if (summary)
> > > {
> > > - if (!modref_may_conflict (summary->stores, ref, tbaa_p))
> > > + if (!modref_may_conflict (call, summary->stores, ref,
> > > tbaa_p))
> > > {
> > > alias_stats.modref_clobber_no_alias++;
> > > if (dump_file && (dump_flags & TDF_DETAILS))
