From: Richard Sandiford <richard.sandiford@arm.com>
To: Richard Biener <richard.guenther@gmail.com>
Cc: Jeff Law <law@redhat.com>, GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: Prevent tree-ssa-dce.c from deleting stores at -Og
Date: Mon, 08 Jul 2019 14:59:00 -0000 [thread overview]
Message-ID: <mpt7e8ssi6b.fsf@arm.com> (raw)
In-Reply-To: <CAFiYyc1VMutW6gFemD5s8gbhAMMzciXmVY_0YaSdSCLVYOavwQ@mail.gmail.com> (Richard Biener's message of "Mon, 8 Jul 2019 13:34:34 +0200")
Richard Biener <richard.guenther@gmail.com> writes:
> On Sun, Jul 7, 2019 at 9:07 PM Jeff Law <law@redhat.com> wrote:
>>
>> On 7/7/19 3:45 AM, Richard Sandiford wrote:
>> > DCE tries to delete dead stores to local data and also tries to insert
>> > debug binds for simple cases:
>> >
>> > /* If this is a store into a variable that is being optimized away,
>> > add a debug bind stmt if possible. */
>> > if (MAY_HAVE_DEBUG_BIND_STMTS
>> > && gimple_assign_single_p (stmt)
>> > && is_gimple_val (gimple_assign_rhs1 (stmt)))
>> > {
>> > tree lhs = gimple_assign_lhs (stmt);
>> > if ((VAR_P (lhs) || TREE_CODE (lhs) == PARM_DECL)
>> > && !DECL_IGNORED_P (lhs)
>> > && is_gimple_reg_type (TREE_TYPE (lhs))
>> > && !is_global_var (lhs)
>> > && !DECL_HAS_VALUE_EXPR_P (lhs))
>> > {
>> > tree rhs = gimple_assign_rhs1 (stmt);
>> > gdebug *note
>> > = gimple_build_debug_bind (lhs, unshare_expr (rhs), stmt);
>> > gsi_insert_after (i, note, GSI_SAME_STMT);
>> > }
>> > }
>> >
>> > But this doesn't help for things like "print *ptr" when ptr points
>> > to the local variable (tests Og-dce-1.c and Og-dce-2.c). It also tends
>> > to make the *live* -- and thus useful -- values optimised out, because
>> > we can't yet switch back to tracking the memory location as it evolves
>> > over time (test Og-dce-3.c).
>> >
>> > So for -Og I think it'd be better not to delete any stmts with
>> > vdefs for now. This also means that we can avoid the potentially
>> > expensive vop walks (which already have a cut-off, but still).
>> >
>> > The patch also fixes the Og failures in gcc.dg/guality/pr54970.c
>> > (PR 86638).
>> >
>> > Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
>> >
>> > Richard
>> >
>> >
>> > 2019-07-07 Richard Sandiford <richard.sandiford@arm.com>
>> >
>> > gcc/
>> > PR debug/86638
>> > * tree-ssa-dce.c (keep_all_vdefs_p): New function.
>> > (mark_stmt_if_obviously_necessary): Mark all stmts with vdefs as
>> > necessary if keep_all_vdefs_p is true.
>> > (mark_aliased_reaching_defs_necessary): Add a gcc_checking_assert
>> > that keep_all_vdefs_p is false.
>> > (mark_all_reaching_defs_necessary): Likewise.
>> > (propagate_necessity): Skip the vuse scan if keep_all_vdefs_p is true.
>> >
>> > gcc/testsuite/
>> > * c-c++-common/guality/Og-dce-1.c: New test.
>> > * c-c++-common/guality/Og-dce-2.c: Likewise.
>> > * c-c++-common/guality/Og-dce-3.c: Likewise.
>> OK
>
> I wonder how code size (and compile-time) is affected by the DSE/DCE patch?
> Say just look at -Og built cc1?
Overall I see a ~2.5% slowdown and a 4.7% increase in load size.
That comes almost entirely from the (RTL) DSE side; this patch
and gimple DSE part don't seem to make much difference.
If I keep the gimple passes as-is and just disable RTL DSE, the slowdown
is still ~2.5% and there's a 4.4% increase in load size.
These are all measuring cc1plus (built from post-patch sources)
and using -O2 -g tree-into-ssa.ii for the speed checks.
> Can you restrict the keep-all-vdefs to user variables (and measure the
> difference this makes)?
In order to avoid wrong debug for pointer dereferences, I think it would
have to be keep-all-vdefs for writes to either user variables or unknown
locations. But as above, I can't measure a significant difference with
the patch.
> Again I wonder if this makes C++ with -Og impractical runtime-wise.
Got a particular test in mind?
Richard
next prev parent reply other threads:[~2019-07-08 14:41 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-07 11:59 Richard Sandiford
2019-07-07 20:13 ` Jeff Law
2019-07-08 11:44 ` Richard Biener
2019-07-08 14:59 ` Richard Sandiford [this message]
2019-07-08 16:32 ` Richard Biener
2019-07-12 11:20 ` Richard Sandiford
2019-07-08 15:06 ` Jeff Law
2019-07-29 9:11 ` Richard Sandiford
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=mpt7e8ssi6b.fsf@arm.com \
--to=richard.sandiford@arm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=law@redhat.com \
--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).