From: Jan Hubicka <hubicka@ucw.cz>
To: Richard Biener <richard.guenther@gmail.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>, d@dcepelik.cz
Subject: Re: Add access through parameter derference tracking to modref
Date: Thu, 24 Sep 2020 12:54:42 +0200 [thread overview]
Message-ID: <20200924105442.GA34649@kam.mff.cuni.cz> (raw)
In-Reply-To: <CAFiYyc0hAvr1wux1CkwLHQbTwsqcpwhQ87pu1p1uK+Bx+HdW6A@mail.gmail.com>
> > + 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)
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))
next prev parent reply other threads:[~2020-09-24 10:54 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-24 9:05 Jan Hubicka
2020-09-24 10:38 ` Richard Biener
2020-09-24 10:54 ` Jan Hubicka [this message]
2020-09-24 11:16 ` Richard Biener
2020-09-24 11:26 ` Jan Hubicka
2020-09-24 11:31 ` Richard Biener
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200924105442.GA34649@kam.mff.cuni.cz \
--to=hubicka@ucw.cz \
--cc=d@dcepelik.cz \
--cc=gcc-patches@gcc.gnu.org \
--cc=richard.guenther@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).