[PATCH] Fix -fsee (PR rtl-optimization/32300)

Jakub Jelinek jakub@redhat.com
Tue Sep 4 19:08:00 GMT 2007


Hi!

-fsee got broken during the dataflow merge in multiple ways.
It creates temporary instructions using copy_rtx (which means
they have the same UID as original and also non-NULL BLOCK_FOR_INSN,
eventhough they aren't in the insn stream yet).

I tried to work with PATTERNs only as suggested in bugzilla, but
that turned out to be too problematic, some insn is really needed for
validate_change etc. and we need to play with REG_NOTES as well;
doing this on the original insn would require undoing all the changes
and redoing them again, and keeping more pointers around (we'd
need PATTERN, REG_NOTES, possibly CALL_INSN_FUNCTION_USAGE
and sequence of the follow-up move insns).

This patch keeps doing basically what the pass did, just in a way
that df groks it, particularly the temporary insn is created
using make_insn_raw, make_jump_insn_raw etc., it is df_insn_delete'd
if not needed (delete_insn would ICE) and the pass no longer tweaks
PREV_INSN/NEXT_INSN chains, instead just emits the temporary insn
into the sequences we are creating anyway.

Clearing DF_DEFER_INSN_RESCAN is also needed for run_fast_dce,
otherwise it ICEs because it assumes no deferal is done,
e.g. the delete_corresponding_reg_eq_notes loop.

Regtested with make -k check RUNTESTFLAGS="--target_board=unix/-fsee"
on x86_64-linux, no regressions from non-fsee make check.

I'm not including any testcases, because basically every second
make check testcase ICEs with -fsee without this patch and it would
be much better if the pass authors could prepare some testcases that
would make sure the pass doesn't break things and also that it
performs the desired optimizations.

Ok for trunk?

2007-09-04  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/32300
	* see.c (see_copy_insn): New function.
	(see_def_extension_not_merged, see_merge_one_use_extension,
	see_merge_one_def_extension): Use it.  Avoid changing
	PREV_INSN/NEXT_INSN chains directly, insted emit insns
	into sequences.  Call df_insn_delete on temporary insns
	that won't be emitted into the insn stream.
	(rest_of_handle_see): Turn off DF_DEFER_INSN_RESCAN
	before run_fast_dce.

--- gcc/see.c.jj	2007-08-13 15:11:18.000000000 +0200
+++ gcc/see.c	2007-09-04 18:59:13.000000000 +0200
@@ -2412,6 +2412,38 @@ see_replace_src (rtx *x, void *data)
 }
 
 
+static rtx
+see_copy_insn (rtx insn)
+{
+  rtx pat = copy_insn (PATTERN (insn)), ret;
+
+  if (NONJUMP_INSN_P (insn))
+    ret = make_insn_raw (pat);
+  else if (JUMP_P (insn))
+    ret = make_jump_insn_raw (pat);
+  else if (CALL_P (insn))
+    {
+      start_sequence ();
+      ret = emit_call_insn (pat);
+      end_sequence ();
+      if (CALL_INSN_FUNCTION_USAGE (insn))
+	CALL_INSN_FUNCTION_USAGE (ret)
+	  = copy_rtx (CALL_INSN_FUNCTION_USAGE (insn));
+      SIBLING_CALL_P (ret) = SIBLING_CALL_P (insn);
+      CONST_OR_PURE_CALL_P (ret) = CONST_OR_PURE_CALL_P (insn);
+    }
+  else
+    gcc_unreachable ();
+  if (REG_NOTES (insn))
+    REG_NOTES (ret) = copy_rtx (REG_NOTES (insn));
+  INSN_LOCATOR (ret) = INSN_LOCATOR (insn);
+  RTX_FRAME_RELATED_P (ret) = RTX_FRAME_RELATED_P (insn);
+  PREV_INSN (ret) = NULL_RTX;
+  NEXT_INSN (ret) = NULL_RTX;
+  return ret;
+}
+
+
 /* At this point the pattern is expected to be:
 
    ref:	    set (dest_reg) (rhs)
@@ -2443,14 +2475,13 @@ see_def_extension_not_merged (struct see
   struct see_replace_data d;
   /* If the original insn was already merged with an extension before,
      take the merged one.  */
-  rtx ref = (curr_ref_s->merged_insn) ? curr_ref_s->merged_insn :
-					curr_ref_s->insn;
-  rtx merged_ref_next = (curr_ref_s->merged_insn) ?
-  			NEXT_INSN (curr_ref_s->merged_insn): NULL_RTX;
-  rtx ref_copy = copy_rtx (ref);
+  rtx ref = curr_ref_s->merged_insn
+	    ? curr_ref_s->merged_insn : curr_ref_s->insn;
+  rtx merged_ref_next = curr_ref_s->merged_insn
+			? NEXT_INSN (curr_ref_s->merged_insn) : NULL_RTX;
+  rtx ref_copy = see_copy_insn (ref);
   rtx source_extension_reg = see_get_extension_reg (def_se, 0);
   rtx dest_extension_reg = see_get_extension_reg (def_se, 1);
-  rtx move_insn = NULL;
   rtx set, rhs;
   rtx dest_reg, dest_real_reg;
   rtx new_pseudo_reg, subreg;
@@ -2489,25 +2520,22 @@ see_def_extension_not_merged (struct see
       || insn_invalid_p (ref_copy))
     {
       /* The manipulation failed.  */
+      df_insn_delete (NULL, INSN_UID (ref_copy));
 
       /* Create a new copy.  */
-      ref_copy = copy_rtx (ref);
+      ref_copy = see_copy_insn (ref);
+
+      if (curr_ref_s->merged_insn)
+	df_insn_delete (NULL, INSN_UID (curr_ref_s->merged_insn));
 
       /* Create a simple move instruction that will replace the def_se.  */
       start_sequence ();
+      emit_insn (ref_copy);
       emit_move_insn (subreg, dest_reg);
-      move_insn = get_insns ();
-      end_sequence ();
-
-      /* Link the manipulated instruction to the newly created move instruction
-	 and to the former created move instructions.  */
-      PREV_INSN (ref_copy) = NULL_RTX;
-      NEXT_INSN (ref_copy) = move_insn;
-      PREV_INSN (move_insn) = ref_copy;
-      NEXT_INSN (move_insn) = merged_ref_next;
       if (merged_ref_next != NULL_RTX)
-	PREV_INSN (merged_ref_next) = move_insn;
-      curr_ref_s->merged_insn = ref_copy;
+	emit_insn (merged_ref_next);
+      curr_ref_s->merged_insn = get_insns ();
+      end_sequence ();
 
       if (dump_file)
 	{
@@ -2516,7 +2544,7 @@ see_def_extension_not_merged (struct see
 	  fprintf (dump_file, "Original ref:\n");
 	  print_rtl_single (dump_file, ref);
 	  fprintf (dump_file, "Move insn that was added:\n");
-	  print_rtl_single (dump_file, move_insn);
+	  print_rtl_single (dump_file, NEXT_INSN (curr_ref_s->merged_insn));
 	}
       return;
     }
@@ -2526,21 +2554,17 @@ see_def_extension_not_merged (struct see
   /* Try to simplify the new manipulated insn.  */
   validate_simplify_insn (ref_copy);
 
+  if (curr_ref_s->merged_insn)
+    df_insn_delete (NULL, INSN_UID (curr_ref_s->merged_insn));
+
   /* Create a simple move instruction to assure the correctness of the code.  */
   start_sequence ();
+  emit_insn (ref_copy);
   emit_move_insn (dest_reg, subreg);
-  move_insn = get_insns ();
-  end_sequence ();
-
-  /* Link the manipulated instruction to the newly created move instruction and
-     to the former created move instructions.  */
-  PREV_INSN (ref_copy) = NULL_RTX;
-  NEXT_INSN (ref_copy) = move_insn;
-  PREV_INSN (move_insn) = ref_copy;
-  NEXT_INSN (move_insn) = merged_ref_next;
   if (merged_ref_next != NULL_RTX)
-    PREV_INSN (merged_ref_next) = move_insn;
-  curr_ref_s->merged_insn = ref_copy;
+    emit_insn (merged_ref_next);
+  curr_ref_s->merged_insn = get_insns ();
+  end_sequence ();
 
   if (dump_file)
     {
@@ -2548,9 +2572,9 @@ see_def_extension_not_merged (struct see
       fprintf (dump_file, "Original ref:\n");
       print_rtl_single (dump_file, ref);
       fprintf (dump_file, "Transformed ref:\n");
-      print_rtl_single (dump_file, ref_copy);
+      print_rtl_single (dump_file, curr_ref_s->merged_insn);
       fprintf (dump_file, "Move insn that was added:\n");
-      print_rtl_single (dump_file, move_insn);
+      print_rtl_single (dump_file, NEXT_INSN (curr_ref_s->merged_insn));
     }
 }
 
@@ -2582,11 +2606,11 @@ see_merge_one_use_extension (void **slot
 {
   struct see_ref_s *curr_ref_s = (struct see_ref_s *) b;
   rtx use_se = *slot;
-  rtx ref = (curr_ref_s->merged_insn) ? curr_ref_s->merged_insn :
-					curr_ref_s->insn;
-  rtx merged_ref_next = (curr_ref_s->merged_insn) ?
-  			NEXT_INSN (curr_ref_s->merged_insn): NULL_RTX;
-  rtx ref_copy = copy_rtx (ref);
+  rtx ref = curr_ref_s->merged_insn
+	    ? curr_ref_s->merged_insn : curr_ref_s->insn;
+  rtx merged_ref_next = curr_ref_s->merged_insn
+			? NEXT_INSN (curr_ref_s->merged_insn) : NULL_RTX;
+  rtx ref_copy = see_copy_insn (ref);
   rtx extension_set = single_set (use_se);
   rtx extension_rhs = NULL;
   rtx dest_extension_reg = see_get_extension_reg (use_se, 1);
@@ -2626,6 +2650,7 @@ see_merge_one_use_extension (void **slot
 	  print_rtl_single (dump_file, use_se);
 	  print_rtl_single (dump_file, ref);
 	}
+      df_insn_delete (NULL, INSN_UID (ref_copy));
       /* Don't remove the current use_se from the use_se_hash and continue to
 	 the next extension.  */
       return 1;
@@ -2644,11 +2669,20 @@ see_merge_one_use_extension (void **slot
 	  print_rtl_single (dump_file, ref);
 	}
       htab_clear_slot (curr_ref_s->use_se_hash, (PTR *)slot);
-      PREV_INSN (ref_copy) = NULL_RTX;
-      NEXT_INSN (ref_copy) = merged_ref_next;
+
+      if (curr_ref_s->merged_insn)
+	df_insn_delete (NULL, INSN_UID (curr_ref_s->merged_insn));
+
       if (merged_ref_next != NULL_RTX)
-	PREV_INSN (merged_ref_next) = ref_copy;
-      curr_ref_s->merged_insn = ref_copy;
+	{
+	  start_sequence ();
+	  emit_insn (ref_copy);
+	  emit_insn (merged_ref_next);
+	  curr_ref_s->merged_insn = get_insns ();
+	  end_sequence ();
+	}
+      else
+	curr_ref_s->merged_insn = ref_copy;
       return 1;
     }
 
@@ -2662,6 +2696,7 @@ see_merge_one_use_extension (void **slot
 	  print_rtl_single (dump_file, use_se);
 	  print_rtl_single (dump_file, ref);
 	}
+      df_insn_delete (NULL, INSN_UID (ref_copy));
       /* Don't remove the current use_se from the use_se_hash and continue to
 	 the next extension.  */
       return 1;
@@ -2672,11 +2707,19 @@ see_merge_one_use_extension (void **slot
   /* Try to simplify the new merged insn.  */
   validate_simplify_insn (ref_copy);
 
-  PREV_INSN (ref_copy) = NULL_RTX;
-  NEXT_INSN (ref_copy) = merged_ref_next;
+  if (curr_ref_s->merged_insn)
+    df_insn_delete (NULL, INSN_UID (curr_ref_s->merged_insn));
+
   if (merged_ref_next != NULL_RTX)
-    PREV_INSN (merged_ref_next) = ref_copy;
-  curr_ref_s->merged_insn = ref_copy;
+    {
+      start_sequence ();
+      emit_insn (ref_copy);
+      emit_insn (merged_ref_next);
+      curr_ref_s->merged_insn = get_insns ();
+      end_sequence ();
+    }
+  else
+    curr_ref_s->merged_insn = ref_copy;
 
   if (dump_file)
     {
@@ -2685,7 +2728,7 @@ see_merge_one_use_extension (void **slot
       print_rtl_single (dump_file, use_se);
       print_rtl_single (dump_file, ref);
       fprintf (dump_file, "Merged instruction:\n");
-      print_rtl_single (dump_file, ref_copy);
+      print_rtl_single (dump_file, curr_ref_s->merged_insn);
     }
 
   /* Remove the current use_se from the use_se_hash.  This will prevent it from
@@ -2726,15 +2769,15 @@ see_merge_one_def_extension (void **slot
   rtx def_se = *slot;
   /* If the original insn was already merged with an extension before,
      take the merged one.  */
-  rtx ref = (curr_ref_s->merged_insn) ? curr_ref_s->merged_insn :
-					curr_ref_s->insn;
-  rtx merged_ref_next = (curr_ref_s->merged_insn) ?
-  			NEXT_INSN (curr_ref_s->merged_insn): NULL_RTX;
-  rtx ref_copy = copy_rtx (ref);
+  rtx ref = curr_ref_s->merged_insn
+	    ? curr_ref_s->merged_insn : curr_ref_s->insn;
+  rtx merged_ref_next = curr_ref_s->merged_insn
+			? NEXT_INSN (curr_ref_s->merged_insn) : NULL_RTX;
+  rtx ref_copy = see_copy_insn (ref);
   rtx new_set = NULL;
   rtx source_extension_reg = see_get_extension_reg (def_se, 0);
   rtx dest_extension_reg = see_get_extension_reg (def_se, 1);
-  rtx move_insn, *rtx_slot, subreg;
+  rtx *rtx_slot, subreg;
   rtx temp_extension = NULL;
   rtx simplified_temp_extension = NULL;
   rtx *pat;
@@ -2763,6 +2806,7 @@ see_merge_one_def_extension (void **slot
 	  print_rtl_single (dump_file, def_se);
 	}
 
+      df_insn_delete (NULL, INSN_UID (ref_copy));
       see_def_extension_not_merged (curr_ref_s, def_se);
       /* Continue to the next extension.  */
       return 1;
@@ -2851,29 +2895,25 @@ see_merge_one_def_extension (void **slot
 	  print_rtl_single (dump_file, def_se);
 	}
 
+      df_insn_delete (NULL, INSN_UID (ref_copy));
       see_def_extension_not_merged (curr_ref_s, def_se);
       /* Continue to the next extension.  */
       return 1;
     }
 
   /* The merge succeeded!  */
+  if (curr_ref_s->merged_insn)
+    df_insn_delete (NULL, INSN_UID (curr_ref_s->merged_insn));
 
   /* Create a simple move instruction to assure the correctness of the code.  */
   subreg = gen_lowpart_SUBREG (source_extension_mode, dest_extension_reg);
   start_sequence ();
+  emit_insn (ref_copy);
   emit_move_insn (source_extension_reg, subreg);
-  move_insn = get_insns ();
-  end_sequence ();
-
-  /* Link the merged instruction to the newly created move instruction and
-     to the former created move instructions.  */
-  PREV_INSN (ref_copy) = NULL_RTX;
-  NEXT_INSN (ref_copy) = move_insn;
-  PREV_INSN (move_insn) = ref_copy;
-  NEXT_INSN (move_insn) = merged_ref_next;
   if (merged_ref_next != NULL_RTX)
-    PREV_INSN (merged_ref_next) = move_insn;
-  curr_ref_s->merged_insn = ref_copy;
+    emit_insn (merged_ref_next);
+  curr_ref_s->merged_insn = get_insns ();
+  end_sequence ();
 
   if (dump_file)
     {
@@ -2882,9 +2922,9 @@ see_merge_one_def_extension (void **slot
       print_rtl_single (dump_file, ref);
       print_rtl_single (dump_file, def_se);
       fprintf (dump_file, "Merged instruction:\n");
-      print_rtl_single (dump_file, ref_copy);
+      print_rtl_single (dump_file, curr_ref_s->merged_insn);
       fprintf (dump_file, "Move instruction that was added:\n");
-      print_rtl_single (dump_file, move_insn);
+      print_rtl_single (dump_file, NEXT_INSN (curr_ref_s->merged_insn));
     }
 
   /* Remove the current def_se from the unmerged_def_se_hash and insert it to
@@ -3814,6 +3854,7 @@ static unsigned int
 rest_of_handle_see (void)
 {
   see_main ();
+  df_clear_flags (DF_DEFER_INSN_RESCAN + DF_NO_INSN_RESCAN);
   run_fast_dce ();
   return 0;
 }

	Jakub



More information about the Gcc-patches mailing list