From: Richard Biener <rguenther@suse.de>
To: kugan <kugan.vivekanandarajah@linaro.org>
Cc: Jan Hubicka <hubicka@ucw.cz>,
"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Subject: Re: [RFC][IPA-VRP] ADDR_EXPR and nonnull
Date: Fri, 21 Oct 2016 08:08:00 -0000 [thread overview]
Message-ID: <alpine.LSU.2.11.1610211006180.2258@t29.fhfr.qr> (raw)
In-Reply-To: <0ef084d3-b9fa-4623-d353-c14a16c0b907@linaro.org>
On Fri, 21 Oct 2016, kugan wrote:
>
>
> On 21/10/16 18:16, Richard Biener wrote:
> > On Fri, 21 Oct 2016, kugan wrote:
> >
> > > Hi,
> > >
> > > On 20/10/16 23:15, Jan Hubicka wrote:
> > > > > Hi Richard,
> > > > >
> > > > >
> > > > > On 20/10/16 18:41, Richard Biener wrote:
> > > > > > On Thu, 20 Oct 2016, kugan wrote:
> > > > > >
> > > > > > >
> > > > > > >
> > > > > > > On 20/10/16 01:26, Jan Hubicka wrote:
> > > > > > > > > > Would excluding weak symbols (I believe I can check
> > > > > > > > > > DECL_WEAK
> > > > > > > > > > for this)
> > > > > > > > > > good
> > > > > > > > > > enough. Or looking for acceptable subset would work?
> > > > > > > > >
> > > > > > > > > I think we should add a symtab helper to tell if
> > > > > > > > > address_nonzero_p
> > > > > > > > > (if
> > > > > > > > > that doesn't aleady exist).
> > > > > > > >
> > > > > > > > We have node->nonzero_address()
> > > > > > >
> > > > > > > Thanks for the pointer. Here is an attempt. Does this look OK?
> > > > > > >
> > > > > > > + if (TREE_CODE (arg) == ADDR_EXPR)
> > > > > > > + {
> > > > > > > + /* See if the AADR_EXPR is nonnull. */
> > > > > > > + varpool_node *node = NULL;
> > > > > > > + tree base = TREE_OPERAND (arg, 0);
> > > > > > > + base = get_base_address (base);
> > > > > > > +
> > > > > > > + if (decl_address_ip_invariant_p (base)
> > > > > > > + || !is_global_var (base))
> > > > > > > + {
> > > > > > > + /* If the symbol address is local or
> > > > > > > + constant. */
> > > > > >
> > > > > > "constant" doesn't matter. You want
> > > > > >
> > > > > > if (TREE_CODE (base) == CONST_DECL
> > > > > > || TREE_CODE (base) == PARM_DECL
> > > > > > || TREE_CODE (base) == RESULT_DECL)
> > > > > > addr_nonzero = true;
> > > > > > else if (VAR_P (base))
> > > > Better to check decl_in_symtab_p (decl)
> > > > > > addr_nonzero = ! is_global_var (base) || (varpool_node::get
> > > > > > (base)->nonzero_address ());
> > > > and symtab_node::get.
> > > >
> > > > I wonder if we can't unify the logic with tree_expr_nonzero_warnv_p
> > > > and corresponding vrp code?
> > > Are you saying that we should export tree_expr_nonzero_warnv_p and use
> > > here
> > > with the logic from above added to tree_expr_nonzero_warnv_p as shown in
> > > the
> > > attached patch?
> >
> > tree_single_nonzero_warnv_p already handles this correctly via
> > maybe_nonzero_address. And tree_single_nonzero_warnv_p is already
> > exported.
> Indeed. maybe_nonzero_address does look at node->nonzero_address () but we
> still seems to miss TREE_CODE (base) == PARM_DECL and TREE_CODE (base) ==
> RESULT_DECL.
Should be handled by
/* Function local objects are never NULL. */
if (DECL_P (base)
&& (DECL_CONTEXT (base)
&& TREE_CODE (DECL_CONTEXT (base)) == FUNCTION_DECL
&& auto_var_in_fn_p (base, DECL_CONTEXT (base))))
return true;
> Does the attached patch looks OK if no regressions?
+ else if (tree_single_nonzero_warnv_p (arg, &strict_overflow))
+ {
+ if (!strict_overflow)
+ addr_nonzero = true;
you don't need to care for strict_overflow here.
The patch is ok with that removed and the fold-const.c changes dropped.
Richard.
> Thanks,
> Kugan
>
>
> >
> > Richard.
> >
> > > Thanks,
> > > Kugan
> > >
> > > >
> > > > Otherwise the patch looks fine to me.
> > > > Honza
> > > >
> > >
> >
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
prev parent reply other threads:[~2016-10-21 8:08 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-19 2:12 kugan
2016-10-19 8:24 ` Richard Biener
2016-10-19 8:48 ` kugan
2016-10-19 9:26 ` Richard Biener
2016-10-19 14:26 ` Jan Hubicka
2016-10-20 3:13 ` kugan
2016-10-20 7:42 ` Richard Biener
2016-10-20 11:56 ` kugan
2016-10-20 12:09 ` Richard Biener
2016-10-20 12:15 ` Jan Hubicka
2016-10-21 3:39 ` kugan
2016-10-21 7:16 ` Richard Biener
2016-10-21 7:38 ` kugan
2016-10-21 8:08 ` Richard Biener [this message]
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=alpine.LSU.2.11.1610211006180.2258@t29.fhfr.qr \
--to=rguenther@suse.de \
--cc=gcc-patches@gcc.gnu.org \
--cc=hubicka@ucw.cz \
--cc=kugan.vivekanandarajah@linaro.org \
/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).