From: Richard Sandiford <richard.sandiford@arm.com>
To: Richard Biener <richard.guenther@gmail.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: Make ifcvt clean up dead comparisons
Date: Fri, 12 Jul 2019 10:32:00 -0000 [thread overview]
Message-ID: <mpt5zo7h7in.fsf@arm.com> (raw)
In-Reply-To: <CAFiYyc3=vw3+XNQZCvPdbRk6Y-nGJ01VDtZDwwpmGsZYcAvx1w@mail.gmail.com> (Richard Biener's message of "Fri, 12 Jul 2019 11:43:55 +0200")
Richard Biener <richard.guenther@gmail.com> writes:
> On Fri, Jul 12, 2019 at 10:00 AM Richard Sandiford
> <richard.sandiford@arm.com> wrote:
>>
>> This change is needed to avoid a regression in gcc.dg/ifcvt-3.c
>> for a later patch. Without it, we enter CSE with a dead comparison left
>> by if-conversion and then eliminate the second (live) comparison in
>> favour of the dead one. That's functionally correct in itself, but it
>> meant that we'd combine the subtraction and comparison into a SUBS
>> before we have a chance to fold away the subtraction.
>>
>> Tested on aarch64-linux-gnu, aarch64_be-elf and x86_64-linux-gnu.
>> OK to install?
>
> cfg_cleanup if it does, runs fast-dce after cleaning up the CFG so does
> it make sense to do this in the caller instead? (and after removing the
> live problem just in case dce tries to keep that updated?)
I think that only happens with CLEANUP_CROSSJUMP, which we don't use
here as things stand. But that could easily change in future, and yeah,
doing it after removing the live problem would be better.
So something like the attached? (Only lightly tested, but it does
still fix the original problem.)
I don't have any evidence that the post-combine pass needs this,
so I've kept it specific to the main pass this time.
Thanks,
Richard
2019-07-12 Richard Sandiford <richard.sandiford@arm.com>
gcc/
* basic-block.h (CLEANUP_FORCE_FAST_DCE): New macro.
* cfgcleanup.c (cleanup_cfg): Call run_fast_dce if
CLEANUP_FORCE_FAST_DCE is set.
* ifcvt.c (rest_of_handle_if_conversion): Pass
CLEANUP_FORCE_FAST_DCE to the final cleanup_cfg call if
if-conversion succeeded.
Index: gcc/basic-block.h
===================================================================
--- gcc/basic-block.h 2019-07-10 19:41:27.171891815 +0100
+++ gcc/basic-block.h 2019-07-12 11:20:59.326008338 +0100
@@ -508,6 +508,8 @@ #define CLEANUP_NO_INSN_DEL 16 /* Do not
#define CLEANUP_CFGLAYOUT 32 /* Do cleanup in cfglayout mode. */
#define CLEANUP_CFG_CHANGED 64 /* The caller changed the CFG. */
#define CLEANUP_NO_PARTITIONING 128 /* Do not try to fix partitions. */
+#define CLEANUP_FORCE_FAST_DCE 0x100 /* Force run_fast_dce to be called
+ at least once. */
/* Return true if BB is in a transaction. */
Index: gcc/cfgcleanup.c
===================================================================
--- gcc/cfgcleanup.c 2019-07-10 19:41:26.395898027 +0100
+++ gcc/cfgcleanup.c 2019-07-12 11:20:59.326008338 +0100
@@ -3193,7 +3193,10 @@ cleanup_cfg (int mode)
&& !delete_trivially_dead_insns (get_insns (), max_reg_num ()))
break;
if ((mode & CLEANUP_CROSSJUMP) && crossjumps_occurred)
- run_fast_dce ();
+ {
+ run_fast_dce ();
+ mode &= ~CLEANUP_FORCE_FAST_DCE;
+ }
}
else
break;
@@ -3202,6 +3205,9 @@ cleanup_cfg (int mode)
if (mode & CLEANUP_CROSSJUMP)
remove_fake_exit_edges ();
+ if (mode & CLEANUP_FORCE_FAST_DCE)
+ run_fast_dce ();
+
/* Don't call delete_dead_jumptables in cfglayout mode, because
that function assumes that jump tables are in the insns stream.
But we also don't _have_ to delete dead jumptables in cfglayout
Index: gcc/ifcvt.c
===================================================================
--- gcc/ifcvt.c 2019-07-12 11:20:55.000000000 +0100
+++ gcc/ifcvt.c 2019-07-12 11:20:59.330008307 +0100
@@ -5457,6 +5457,8 @@ if_convert (bool after_combine)
static unsigned int
rest_of_handle_if_conversion (void)
{
+ int flags = 0;
+
if (flag_if_conversion)
{
if (dump_file)
@@ -5466,9 +5468,12 @@ rest_of_handle_if_conversion (void)
}
cleanup_cfg (CLEANUP_EXPENSIVE);
if_convert (false);
+ if (num_updated_if_blocks)
+ /* Get rid of any dead CC-related instructions. */
+ flags |= CLEANUP_FORCE_FAST_DCE;
}
- cleanup_cfg (0);
+ cleanup_cfg (flags);
return 0;
}
next prev parent reply other threads:[~2019-07-12 10:28 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-12 8:08 Richard Sandiford
2019-07-12 9:53 ` Richard Biener
2019-07-12 10:32 ` Richard Sandiford [this message]
2019-07-12 10:36 ` Richard Biener
2019-07-12 10:49 ` 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=mpt5zo7h7in.fsf@arm.com \
--to=richard.sandiford@arm.com \
--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).