Richard Biener <richard.guent...@gmail.com> writes:
> On Fri, Jul 12, 2019 at 10:00 AM Richard Sandiford
> <richard.sandif...@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.sandif...@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;
}
