Make ifcvt clean up dead comparisons

Richard Sandiford richard.sandiford@arm.com
Fri Jul 12 10:32:00 GMT 2019


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;
 }
 



More information about the Gcc-patches mailing list