public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][RFC] Remove RTL loop unswitching
@ 2014-04-15  9:28 Richard Biener
  2014-04-15 12:58 ` Richard Biener
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Richard Biener @ 2014-04-15  9:28 UTC (permalink / raw)
  To: gcc-patches


This removes RTL loop unswitching (see last years discussion about
compile-time issues of that pass).  RTL loop unswitching is
enabled together with GIMPLE loop unswitching at -O3 and by
-floop-unswitch.  It's clearly the wrong place to do high-level
loop transforms these days, and the cost of maintainance doesn't
outweight the questionable benefit.

Thus the following patch removes it.

Bootstrap / regtest pending on x86_64-unknown-linux-gnu (I hope
for testsuite fallout).

Any objections?

Thanks,
Richard.

2014-04-15  Richard Biener  <rguenther@suse.de>

	* Makefile.in (OBJS): Remove loop-unswitch.o.
	* loop-unswitch.c: Delete.
	* tree-pass.h (make_pass_rtl_unswitch): Remove.
	* passes.def (pass_rtl_unswitch): Likewise.
	* loop-init.c (gate_rtl_unswitch): Likewise.
	(rtl_unswitch): Likewise.
	(pass_data_rtl_unswitch): Likewise.
	(pass_rtl_unswitch): Likewise.
	(make_pass_rtl_unswitch): Likewise.
	* rtl.h (reversed_condition): Likewise.
	(compare_and_jump_seq): Likewise.
	* loop-iv.c (reversed_condition): Move here from loop-unswitch.c
	and make static.
	* loop-unroll.c (compare_and_jump_seq): Likewise.

Index: gcc/Makefile.in
===================================================================
--- gcc/Makefile.in	(revision 209410)
+++ gcc/Makefile.in	(working copy)
@@ -1294,7 +1294,6 @@ OBJS = \
 	loop-invariant.o \
 	loop-iv.o \
 	loop-unroll.o \
-	loop-unswitch.o \
 	lower-subreg.o \
 	lra.o \
 	lra-assigns.o \
Index: gcc/tree-pass.h
===================================================================
--- gcc/tree-pass.h	(revision 209410)
+++ gcc/tree-pass.h	(working copy)
@@ -512,7 +512,6 @@ extern rtl_opt_pass *make_pass_outof_cfg
 extern rtl_opt_pass *make_pass_loop2 (gcc::context *ctxt);
 extern rtl_opt_pass *make_pass_rtl_loop_init (gcc::context *ctxt);
 extern rtl_opt_pass *make_pass_rtl_move_loop_invariants (gcc::context *ctxt);
-extern rtl_opt_pass *make_pass_rtl_unswitch (gcc::context *ctxt);
 extern rtl_opt_pass *make_pass_rtl_unroll_and_peel_loops (gcc::context *ctxt);
 extern rtl_opt_pass *make_pass_rtl_doloop (gcc::context *ctxt);
 extern rtl_opt_pass *make_pass_rtl_loop_done (gcc::context *ctxt);
Index: gcc/passes.def
===================================================================
--- gcc/passes.def	(revision 209410)
+++ gcc/passes.def	(working copy)
@@ -341,7 +341,6 @@ along with GCC; see the file COPYING3.
       PUSH_INSERT_PASSES_WITHIN (pass_loop2)
 	  NEXT_PASS (pass_rtl_loop_init);
 	  NEXT_PASS (pass_rtl_move_loop_invariants);
-	  NEXT_PASS (pass_rtl_unswitch);
 	  NEXT_PASS (pass_rtl_unroll_and_peel_loops);
 	  NEXT_PASS (pass_rtl_doloop);
 	  NEXT_PASS (pass_rtl_loop_done);
Index: gcc/loop-init.c
===================================================================
--- gcc/loop-init.c	(revision 209410)
+++ gcc/loop-init.c	(working copy)
@@ -518,61 +518,7 @@ make_pass_rtl_move_loop_invariants (gcc:
 }
 
 \f
-/* Loop unswitching for RTL.  */
-static bool
-gate_rtl_unswitch (void)
-{
-  return flag_unswitch_loops;
-}
-
-static unsigned int
-rtl_unswitch (void)
-{
-  if (number_of_loops (cfun) > 1)
-    unswitch_loops ();
-  return 0;
-}
-
-namespace {
-
-const pass_data pass_data_rtl_unswitch =
-{
-  RTL_PASS, /* type */
-  "loop2_unswitch", /* name */
-  OPTGROUP_LOOP, /* optinfo_flags */
-  true, /* has_gate */
-  true, /* has_execute */
-  TV_LOOP_UNSWITCH, /* tv_id */
-  0, /* properties_required */
-  0, /* properties_provided */
-  0, /* properties_destroyed */
-  0, /* todo_flags_start */
-  TODO_verify_rtl_sharing, /* todo_flags_finish */
-};
-
-class pass_rtl_unswitch : public rtl_opt_pass
-{
-public:
-  pass_rtl_unswitch (gcc::context *ctxt)
-    : rtl_opt_pass (pass_data_rtl_unswitch, ctxt)
-  {}
-
-  /* opt_pass methods: */
-  bool gate () { return gate_rtl_unswitch (); }
-  unsigned int execute () { return rtl_unswitch (); }
-
-}; // class pass_rtl_unswitch
-
-} // anon namespace
-
-rtl_opt_pass *
-make_pass_rtl_unswitch (gcc::context *ctxt)
-{
-  return new pass_rtl_unswitch (ctxt);
-}
-
-\f
-/* Loop unswitching for RTL.  */
+/* Loop unrolling and peeling for RTL.  */
 static bool
 gate_rtl_unroll_and_peel_loops (void)
 {
Index: gcc/loop-iv.c
===================================================================
--- gcc/loop-iv.c	(revision 209410)
+++ gcc/loop-iv.c	(working copy)
@@ -1732,6 +1732,21 @@ canon_condition (rtx cond)
   return cond;
 }
 
+/* Reverses CONDition; returns NULL if we cannot.  */
+
+static rtx
+reversed_condition (rtx cond)
+{
+  enum rtx_code reversed;
+  reversed = reversed_comparison_code (cond, NULL);
+  if (reversed == UNKNOWN)
+    return NULL_RTX;
+  else
+    return gen_rtx_fmt_ee (reversed,
+			   GET_MODE (cond), XEXP (cond, 0),
+			   XEXP (cond, 1));
+}
+
 /* Tries to use the fact that COND holds to simplify EXPR.  ALTERED is the
    set of altered regs.  */
 
Index: gcc/loop-unroll.c
===================================================================
--- gcc/loop-unroll.c	(revision 209410)
+++ gcc/loop-unroll.c	(working copy)
@@ -1060,6 +1060,59 @@ split_edge_and_insert (edge e, rtx insns
   return bb;
 }
 
+/* Prepare a sequence comparing OP0 with OP1 using COMP and jumping to LABEL if
+   true, with probability PROB.  If CINSN is not NULL, it is the insn to copy
+   in order to create a jump.  */
+
+static rtx
+compare_and_jump_seq (rtx op0, rtx op1, enum rtx_code comp, rtx label, int prob,
+		      rtx cinsn)
+{
+  rtx seq, jump, cond;
+  enum machine_mode mode;
+
+  mode = GET_MODE (op0);
+  if (mode == VOIDmode)
+    mode = GET_MODE (op1);
+
+  start_sequence ();
+  if (GET_MODE_CLASS (mode) == MODE_CC)
+    {
+      /* A hack -- there seems to be no easy generic way how to make a
+	 conditional jump from a ccmode comparison.  */
+      gcc_assert (cinsn);
+      cond = XEXP (SET_SRC (pc_set (cinsn)), 0);
+      gcc_assert (GET_CODE (cond) == comp);
+      gcc_assert (rtx_equal_p (op0, XEXP (cond, 0)));
+      gcc_assert (rtx_equal_p (op1, XEXP (cond, 1)));
+      emit_jump_insn (copy_insn (PATTERN (cinsn)));
+      jump = get_last_insn ();
+      gcc_assert (JUMP_P (jump));
+      JUMP_LABEL (jump) = JUMP_LABEL (cinsn);
+      LABEL_NUSES (JUMP_LABEL (jump))++;
+      redirect_jump (jump, label, 0);
+    }
+  else
+    {
+      gcc_assert (!cinsn);
+
+      op0 = force_operand (op0, NULL_RTX);
+      op1 = force_operand (op1, NULL_RTX);
+      do_compare_rtx_and_jump (op0, op1, comp, 0,
+			       mode, NULL_RTX, NULL_RTX, label, -1);
+      jump = get_last_insn ();
+      gcc_assert (JUMP_P (jump));
+      JUMP_LABEL (jump) = label;
+      LABEL_NUSES (label)++;
+    }
+  add_int_reg_note (jump, REG_BR_PROB, prob);
+
+  seq = get_insns ();
+  end_sequence ();
+
+  return seq;
+}
+
 /* Unroll LOOP for which we are able to count number of iterations in runtime
    LOOP->LPT_DECISION.TIMES times.  The transformation does this (with some
    extra care for case n < 0):
Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h	(revision 209410)
+++ gcc/rtl.h	(working copy)
@@ -2743,10 +2743,6 @@ extern unsigned int variable_tracking_ma
 extern void get_mode_bounds (enum machine_mode, int, enum machine_mode,
 			     rtx *, rtx *);
 
-/* In loop-unswitch.c  */
-extern rtx reversed_condition (rtx);
-extern rtx compare_and_jump_seq (rtx, rtx, enum rtx_code, rtx, int, rtx);
-
 /* In loop-iv.c  */
 extern rtx canon_condition (rtx);
 extern void simplify_using_condition (rtx, rtx *, bitmap);

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][RFC] Remove RTL loop unswitching
  2014-04-15  9:28 [PATCH][RFC] Remove RTL loop unswitching Richard Biener
@ 2014-04-15 12:58 ` Richard Biener
  2014-04-20  7:09 ` Jan Hubicka
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Richard Biener @ 2014-04-15 12:58 UTC (permalink / raw)
  To: gcc-patches

On Tue, 15 Apr 2014, Richard Biener wrote:

> 
> This removes RTL loop unswitching (see last years discussion about
> compile-time issues of that pass).  RTL loop unswitching is
> enabled together with GIMPLE loop unswitching at -O3 and by
> -floop-unswitch.  It's clearly the wrong place to do high-level
> loop transforms these days, and the cost of maintainance doesn't
> outweight the questionable benefit.
> 
> Thus the following patch removes it.
> 
> Bootstrap / regtest pending on x86_64-unknown-linux-gnu (I hope
> for testsuite fallout).

No testsuite fallout, thus no testcases that test for a working
RTL unswitching (on x86_64/i586 at least).

Richard.

> Any objections?
> 
> Thanks,
> Richard.
> 
> 2014-04-15  Richard Biener  <rguenther@suse.de>
> 
> 	* Makefile.in (OBJS): Remove loop-unswitch.o.
> 	* loop-unswitch.c: Delete.
> 	* tree-pass.h (make_pass_rtl_unswitch): Remove.
> 	* passes.def (pass_rtl_unswitch): Likewise.
> 	* loop-init.c (gate_rtl_unswitch): Likewise.
> 	(rtl_unswitch): Likewise.
> 	(pass_data_rtl_unswitch): Likewise.
> 	(pass_rtl_unswitch): Likewise.
> 	(make_pass_rtl_unswitch): Likewise.
> 	* rtl.h (reversed_condition): Likewise.
> 	(compare_and_jump_seq): Likewise.
> 	* loop-iv.c (reversed_condition): Move here from loop-unswitch.c
> 	and make static.
> 	* loop-unroll.c (compare_and_jump_seq): Likewise.
> 
> Index: gcc/Makefile.in
> ===================================================================
> --- gcc/Makefile.in	(revision 209410)
> +++ gcc/Makefile.in	(working copy)
> @@ -1294,7 +1294,6 @@ OBJS = \
>  	loop-invariant.o \
>  	loop-iv.o \
>  	loop-unroll.o \
> -	loop-unswitch.o \
>  	lower-subreg.o \
>  	lra.o \
>  	lra-assigns.o \
> Index: gcc/tree-pass.h
> ===================================================================
> --- gcc/tree-pass.h	(revision 209410)
> +++ gcc/tree-pass.h	(working copy)
> @@ -512,7 +512,6 @@ extern rtl_opt_pass *make_pass_outof_cfg
>  extern rtl_opt_pass *make_pass_loop2 (gcc::context *ctxt);
>  extern rtl_opt_pass *make_pass_rtl_loop_init (gcc::context *ctxt);
>  extern rtl_opt_pass *make_pass_rtl_move_loop_invariants (gcc::context *ctxt);
> -extern rtl_opt_pass *make_pass_rtl_unswitch (gcc::context *ctxt);
>  extern rtl_opt_pass *make_pass_rtl_unroll_and_peel_loops (gcc::context *ctxt);
>  extern rtl_opt_pass *make_pass_rtl_doloop (gcc::context *ctxt);
>  extern rtl_opt_pass *make_pass_rtl_loop_done (gcc::context *ctxt);
> Index: gcc/passes.def
> ===================================================================
> --- gcc/passes.def	(revision 209410)
> +++ gcc/passes.def	(working copy)
> @@ -341,7 +341,6 @@ along with GCC; see the file COPYING3.
>        PUSH_INSERT_PASSES_WITHIN (pass_loop2)
>  	  NEXT_PASS (pass_rtl_loop_init);
>  	  NEXT_PASS (pass_rtl_move_loop_invariants);
> -	  NEXT_PASS (pass_rtl_unswitch);
>  	  NEXT_PASS (pass_rtl_unroll_and_peel_loops);
>  	  NEXT_PASS (pass_rtl_doloop);
>  	  NEXT_PASS (pass_rtl_loop_done);
> Index: gcc/loop-init.c
> ===================================================================
> --- gcc/loop-init.c	(revision 209410)
> +++ gcc/loop-init.c	(working copy)
> @@ -518,61 +518,7 @@ make_pass_rtl_move_loop_invariants (gcc:
>  }
>  
>  \f
> -/* Loop unswitching for RTL.  */
> -static bool
> -gate_rtl_unswitch (void)
> -{
> -  return flag_unswitch_loops;
> -}
> -
> -static unsigned int
> -rtl_unswitch (void)
> -{
> -  if (number_of_loops (cfun) > 1)
> -    unswitch_loops ();
> -  return 0;
> -}
> -
> -namespace {
> -
> -const pass_data pass_data_rtl_unswitch =
> -{
> -  RTL_PASS, /* type */
> -  "loop2_unswitch", /* name */
> -  OPTGROUP_LOOP, /* optinfo_flags */
> -  true, /* has_gate */
> -  true, /* has_execute */
> -  TV_LOOP_UNSWITCH, /* tv_id */
> -  0, /* properties_required */
> -  0, /* properties_provided */
> -  0, /* properties_destroyed */
> -  0, /* todo_flags_start */
> -  TODO_verify_rtl_sharing, /* todo_flags_finish */
> -};
> -
> -class pass_rtl_unswitch : public rtl_opt_pass
> -{
> -public:
> -  pass_rtl_unswitch (gcc::context *ctxt)
> -    : rtl_opt_pass (pass_data_rtl_unswitch, ctxt)
> -  {}
> -
> -  /* opt_pass methods: */
> -  bool gate () { return gate_rtl_unswitch (); }
> -  unsigned int execute () { return rtl_unswitch (); }
> -
> -}; // class pass_rtl_unswitch
> -
> -} // anon namespace
> -
> -rtl_opt_pass *
> -make_pass_rtl_unswitch (gcc::context *ctxt)
> -{
> -  return new pass_rtl_unswitch (ctxt);
> -}
> -
> -\f
> -/* Loop unswitching for RTL.  */
> +/* Loop unrolling and peeling for RTL.  */
>  static bool
>  gate_rtl_unroll_and_peel_loops (void)
>  {
> Index: gcc/loop-iv.c
> ===================================================================
> --- gcc/loop-iv.c	(revision 209410)
> +++ gcc/loop-iv.c	(working copy)
> @@ -1732,6 +1732,21 @@ canon_condition (rtx cond)
>    return cond;
>  }
>  
> +/* Reverses CONDition; returns NULL if we cannot.  */
> +
> +static rtx
> +reversed_condition (rtx cond)
> +{
> +  enum rtx_code reversed;
> +  reversed = reversed_comparison_code (cond, NULL);
> +  if (reversed == UNKNOWN)
> +    return NULL_RTX;
> +  else
> +    return gen_rtx_fmt_ee (reversed,
> +			   GET_MODE (cond), XEXP (cond, 0),
> +			   XEXP (cond, 1));
> +}
> +
>  /* Tries to use the fact that COND holds to simplify EXPR.  ALTERED is the
>     set of altered regs.  */
>  
> Index: gcc/loop-unroll.c
> ===================================================================
> --- gcc/loop-unroll.c	(revision 209410)
> +++ gcc/loop-unroll.c	(working copy)
> @@ -1060,6 +1060,59 @@ split_edge_and_insert (edge e, rtx insns
>    return bb;
>  }
>  
> +/* Prepare a sequence comparing OP0 with OP1 using COMP and jumping to LABEL if
> +   true, with probability PROB.  If CINSN is not NULL, it is the insn to copy
> +   in order to create a jump.  */
> +
> +static rtx
> +compare_and_jump_seq (rtx op0, rtx op1, enum rtx_code comp, rtx label, int prob,
> +		      rtx cinsn)
> +{
> +  rtx seq, jump, cond;
> +  enum machine_mode mode;
> +
> +  mode = GET_MODE (op0);
> +  if (mode == VOIDmode)
> +    mode = GET_MODE (op1);
> +
> +  start_sequence ();
> +  if (GET_MODE_CLASS (mode) == MODE_CC)
> +    {
> +      /* A hack -- there seems to be no easy generic way how to make a
> +	 conditional jump from a ccmode comparison.  */
> +      gcc_assert (cinsn);
> +      cond = XEXP (SET_SRC (pc_set (cinsn)), 0);
> +      gcc_assert (GET_CODE (cond) == comp);
> +      gcc_assert (rtx_equal_p (op0, XEXP (cond, 0)));
> +      gcc_assert (rtx_equal_p (op1, XEXP (cond, 1)));
> +      emit_jump_insn (copy_insn (PATTERN (cinsn)));
> +      jump = get_last_insn ();
> +      gcc_assert (JUMP_P (jump));
> +      JUMP_LABEL (jump) = JUMP_LABEL (cinsn);
> +      LABEL_NUSES (JUMP_LABEL (jump))++;
> +      redirect_jump (jump, label, 0);
> +    }
> +  else
> +    {
> +      gcc_assert (!cinsn);
> +
> +      op0 = force_operand (op0, NULL_RTX);
> +      op1 = force_operand (op1, NULL_RTX);
> +      do_compare_rtx_and_jump (op0, op1, comp, 0,
> +			       mode, NULL_RTX, NULL_RTX, label, -1);
> +      jump = get_last_insn ();
> +      gcc_assert (JUMP_P (jump));
> +      JUMP_LABEL (jump) = label;
> +      LABEL_NUSES (label)++;
> +    }
> +  add_int_reg_note (jump, REG_BR_PROB, prob);
> +
> +  seq = get_insns ();
> +  end_sequence ();
> +
> +  return seq;
> +}
> +
>  /* Unroll LOOP for which we are able to count number of iterations in runtime
>     LOOP->LPT_DECISION.TIMES times.  The transformation does this (with some
>     extra care for case n < 0):
> Index: gcc/rtl.h
> ===================================================================
> --- gcc/rtl.h	(revision 209410)
> +++ gcc/rtl.h	(working copy)
> @@ -2743,10 +2743,6 @@ extern unsigned int variable_tracking_ma
>  extern void get_mode_bounds (enum machine_mode, int, enum machine_mode,
>  			     rtx *, rtx *);
>  
> -/* In loop-unswitch.c  */
> -extern rtx reversed_condition (rtx);
> -extern rtx compare_and_jump_seq (rtx, rtx, enum rtx_code, rtx, int, rtx);
> -
>  /* In loop-iv.c  */
>  extern rtx canon_condition (rtx);
>  extern void simplify_using_condition (rtx, rtx *, bitmap);
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][RFC] Remove RTL loop unswitching
  2014-04-15  9:28 [PATCH][RFC] Remove RTL loop unswitching Richard Biener
  2014-04-15 12:58 ` Richard Biener
@ 2014-04-20  7:09 ` Jan Hubicka
  2014-04-23  8:20   ` Richard Biener
  2014-05-07 10:34 ` Thomas Schwinge
  2014-05-07 12:45 ` Thomas Schwinge
  3 siblings, 1 reply; 8+ messages in thread
From: Jan Hubicka @ 2014-04-20  7:09 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

> 
> This removes RTL loop unswitching (see last years discussion about
> compile-time issues of that pass).  RTL loop unswitching is
> enabled together with GIMPLE loop unswitching at -O3 and by
> -floop-unswitch.  It's clearly the wrong place to do high-level
> loop transforms these days, and the cost of maintainance doesn't
> outweight the questionable benefit.
> 
> Thus the following patch removes it.
> 
> Bootstrap / regtest pending on x86_64-unknown-linux-gnu (I hope
> for testsuite fallout).
> 
> Any objections?

Not really, I am all for moving more of loop stuff to trees.
Did you performed some benchmarks? (I remember I did in 2012
but completely forgot the outcome).

On related note, shall I try to update the following?
http://gcc.gnu.org/ml/gcc-patches/2013-11/msg02270.html

Honza
> 
> Thanks,
> Richard.
> 
> 2014-04-15  Richard Biener  <rguenther@suse.de>
> 
> 	* Makefile.in (OBJS): Remove loop-unswitch.o.
> 	* loop-unswitch.c: Delete.
> 	* tree-pass.h (make_pass_rtl_unswitch): Remove.
> 	* passes.def (pass_rtl_unswitch): Likewise.
> 	* loop-init.c (gate_rtl_unswitch): Likewise.
> 	(rtl_unswitch): Likewise.
> 	(pass_data_rtl_unswitch): Likewise.
> 	(pass_rtl_unswitch): Likewise.
> 	(make_pass_rtl_unswitch): Likewise.
> 	* rtl.h (reversed_condition): Likewise.
> 	(compare_and_jump_seq): Likewise.
> 	* loop-iv.c (reversed_condition): Move here from loop-unswitch.c
> 	and make static.
> 	* loop-unroll.c (compare_and_jump_seq): Likewise.
> 
> Index: gcc/Makefile.in
> ===================================================================
> --- gcc/Makefile.in	(revision 209410)
> +++ gcc/Makefile.in	(working copy)
> @@ -1294,7 +1294,6 @@ OBJS = \
>  	loop-invariant.o \
>  	loop-iv.o \
>  	loop-unroll.o \
> -	loop-unswitch.o \
>  	lower-subreg.o \
>  	lra.o \
>  	lra-assigns.o \
> Index: gcc/tree-pass.h
> ===================================================================
> --- gcc/tree-pass.h	(revision 209410)
> +++ gcc/tree-pass.h	(working copy)
> @@ -512,7 +512,6 @@ extern rtl_opt_pass *make_pass_outof_cfg
>  extern rtl_opt_pass *make_pass_loop2 (gcc::context *ctxt);
>  extern rtl_opt_pass *make_pass_rtl_loop_init (gcc::context *ctxt);
>  extern rtl_opt_pass *make_pass_rtl_move_loop_invariants (gcc::context *ctxt);
> -extern rtl_opt_pass *make_pass_rtl_unswitch (gcc::context *ctxt);
>  extern rtl_opt_pass *make_pass_rtl_unroll_and_peel_loops (gcc::context *ctxt);
>  extern rtl_opt_pass *make_pass_rtl_doloop (gcc::context *ctxt);
>  extern rtl_opt_pass *make_pass_rtl_loop_done (gcc::context *ctxt);
> Index: gcc/passes.def
> ===================================================================
> --- gcc/passes.def	(revision 209410)
> +++ gcc/passes.def	(working copy)
> @@ -341,7 +341,6 @@ along with GCC; see the file COPYING3.
>        PUSH_INSERT_PASSES_WITHIN (pass_loop2)
>  	  NEXT_PASS (pass_rtl_loop_init);
>  	  NEXT_PASS (pass_rtl_move_loop_invariants);
> -	  NEXT_PASS (pass_rtl_unswitch);
>  	  NEXT_PASS (pass_rtl_unroll_and_peel_loops);
>  	  NEXT_PASS (pass_rtl_doloop);
>  	  NEXT_PASS (pass_rtl_loop_done);
> Index: gcc/loop-init.c
> ===================================================================
> --- gcc/loop-init.c	(revision 209410)
> +++ gcc/loop-init.c	(working copy)
> @@ -518,61 +518,7 @@ make_pass_rtl_move_loop_invariants (gcc:
>  }
>  
>  \f
> -/* Loop unswitching for RTL.  */
> -static bool
> -gate_rtl_unswitch (void)
> -{
> -  return flag_unswitch_loops;
> -}
> -
> -static unsigned int
> -rtl_unswitch (void)
> -{
> -  if (number_of_loops (cfun) > 1)
> -    unswitch_loops ();
> -  return 0;
> -}
> -
> -namespace {
> -
> -const pass_data pass_data_rtl_unswitch =
> -{
> -  RTL_PASS, /* type */
> -  "loop2_unswitch", /* name */
> -  OPTGROUP_LOOP, /* optinfo_flags */
> -  true, /* has_gate */
> -  true, /* has_execute */
> -  TV_LOOP_UNSWITCH, /* tv_id */
> -  0, /* properties_required */
> -  0, /* properties_provided */
> -  0, /* properties_destroyed */
> -  0, /* todo_flags_start */
> -  TODO_verify_rtl_sharing, /* todo_flags_finish */
> -};
> -
> -class pass_rtl_unswitch : public rtl_opt_pass
> -{
> -public:
> -  pass_rtl_unswitch (gcc::context *ctxt)
> -    : rtl_opt_pass (pass_data_rtl_unswitch, ctxt)
> -  {}
> -
> -  /* opt_pass methods: */
> -  bool gate () { return gate_rtl_unswitch (); }
> -  unsigned int execute () { return rtl_unswitch (); }
> -
> -}; // class pass_rtl_unswitch
> -
> -} // anon namespace
> -
> -rtl_opt_pass *
> -make_pass_rtl_unswitch (gcc::context *ctxt)
> -{
> -  return new pass_rtl_unswitch (ctxt);
> -}
> -
> -\f
> -/* Loop unswitching for RTL.  */
> +/* Loop unrolling and peeling for RTL.  */
>  static bool
>  gate_rtl_unroll_and_peel_loops (void)
>  {
> Index: gcc/loop-iv.c
> ===================================================================
> --- gcc/loop-iv.c	(revision 209410)
> +++ gcc/loop-iv.c	(working copy)
> @@ -1732,6 +1732,21 @@ canon_condition (rtx cond)
>    return cond;
>  }
>  
> +/* Reverses CONDition; returns NULL if we cannot.  */
> +
> +static rtx
> +reversed_condition (rtx cond)
> +{
> +  enum rtx_code reversed;
> +  reversed = reversed_comparison_code (cond, NULL);
> +  if (reversed == UNKNOWN)
> +    return NULL_RTX;
> +  else
> +    return gen_rtx_fmt_ee (reversed,
> +			   GET_MODE (cond), XEXP (cond, 0),
> +			   XEXP (cond, 1));
> +}
> +
>  /* Tries to use the fact that COND holds to simplify EXPR.  ALTERED is the
>     set of altered regs.  */
>  
> Index: gcc/loop-unroll.c
> ===================================================================
> --- gcc/loop-unroll.c	(revision 209410)
> +++ gcc/loop-unroll.c	(working copy)
> @@ -1060,6 +1060,59 @@ split_edge_and_insert (edge e, rtx insns
>    return bb;
>  }
>  
> +/* Prepare a sequence comparing OP0 with OP1 using COMP and jumping to LABEL if
> +   true, with probability PROB.  If CINSN is not NULL, it is the insn to copy
> +   in order to create a jump.  */
> +
> +static rtx
> +compare_and_jump_seq (rtx op0, rtx op1, enum rtx_code comp, rtx label, int prob,
> +		      rtx cinsn)
> +{
> +  rtx seq, jump, cond;
> +  enum machine_mode mode;
> +
> +  mode = GET_MODE (op0);
> +  if (mode == VOIDmode)
> +    mode = GET_MODE (op1);
> +
> +  start_sequence ();
> +  if (GET_MODE_CLASS (mode) == MODE_CC)
> +    {
> +      /* A hack -- there seems to be no easy generic way how to make a
> +	 conditional jump from a ccmode comparison.  */
> +      gcc_assert (cinsn);
> +      cond = XEXP (SET_SRC (pc_set (cinsn)), 0);
> +      gcc_assert (GET_CODE (cond) == comp);
> +      gcc_assert (rtx_equal_p (op0, XEXP (cond, 0)));
> +      gcc_assert (rtx_equal_p (op1, XEXP (cond, 1)));
> +      emit_jump_insn (copy_insn (PATTERN (cinsn)));
> +      jump = get_last_insn ();
> +      gcc_assert (JUMP_P (jump));
> +      JUMP_LABEL (jump) = JUMP_LABEL (cinsn);
> +      LABEL_NUSES (JUMP_LABEL (jump))++;
> +      redirect_jump (jump, label, 0);
> +    }
> +  else
> +    {
> +      gcc_assert (!cinsn);
> +
> +      op0 = force_operand (op0, NULL_RTX);
> +      op1 = force_operand (op1, NULL_RTX);
> +      do_compare_rtx_and_jump (op0, op1, comp, 0,
> +			       mode, NULL_RTX, NULL_RTX, label, -1);
> +      jump = get_last_insn ();
> +      gcc_assert (JUMP_P (jump));
> +      JUMP_LABEL (jump) = label;
> +      LABEL_NUSES (label)++;
> +    }
> +  add_int_reg_note (jump, REG_BR_PROB, prob);
> +
> +  seq = get_insns ();
> +  end_sequence ();
> +
> +  return seq;
> +}
> +
>  /* Unroll LOOP for which we are able to count number of iterations in runtime
>     LOOP->LPT_DECISION.TIMES times.  The transformation does this (with some
>     extra care for case n < 0):
> Index: gcc/rtl.h
> ===================================================================
> --- gcc/rtl.h	(revision 209410)
> +++ gcc/rtl.h	(working copy)
> @@ -2743,10 +2743,6 @@ extern unsigned int variable_tracking_ma
>  extern void get_mode_bounds (enum machine_mode, int, enum machine_mode,
>  			     rtx *, rtx *);
>  
> -/* In loop-unswitch.c  */
> -extern rtx reversed_condition (rtx);
> -extern rtx compare_and_jump_seq (rtx, rtx, enum rtx_code, rtx, int, rtx);
> -
>  /* In loop-iv.c  */
>  extern rtx canon_condition (rtx);
>  extern void simplify_using_condition (rtx, rtx *, bitmap);

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][RFC] Remove RTL loop unswitching
  2014-04-20  7:09 ` Jan Hubicka
@ 2014-04-23  8:20   ` Richard Biener
  2014-04-23 16:32     ` Jan Hubicka
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Biener @ 2014-04-23  8:20 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: gcc-patches

On Sun, 20 Apr 2014, Jan Hubicka wrote:

> > 
> > This removes RTL loop unswitching (see last years discussion about
> > compile-time issues of that pass).  RTL loop unswitching is
> > enabled together with GIMPLE loop unswitching at -O3 and by
> > -floop-unswitch.  It's clearly the wrong place to do high-level
> > loop transforms these days, and the cost of maintainance doesn't
> > outweight the questionable benefit.
> > 
> > Thus the following patch removes it.
> > 
> > Bootstrap / regtest pending on x86_64-unknown-linux-gnu (I hope
> > for testsuite fallout).
> > 
> > Any objections?
> 
> Not really, I am all for moving more of loop stuff to trees.
> Did you performed some benchmarks? (I remember I did in 2012
> but completely forgot the outcome).

I did that last year and it showed no difference in SPEC 2k6.

When bootstrapping with -O3 and a gcc_unreachable () in the
RTL unswitching path you get some ICEs there but they are
due to different "effective" --param max-unswitch-insns that
is on GIMPLE applied to tree_num_loop_insns () and on RTL
to num_loop_insns ().

I'll go forward with the patch today.

> On related note, shall I try to update the following?
> http://gcc.gnu.org/ml/gcc-patches/2013-11/msg02270.html

Yeah.

Thanks,
Richard.

> Honza
> > 
> > Thanks,
> > Richard.
> > 
> > 2014-04-15  Richard Biener  <rguenther@suse.de>
> > 
> > 	* Makefile.in (OBJS): Remove loop-unswitch.o.
> > 	* loop-unswitch.c: Delete.
> > 	* tree-pass.h (make_pass_rtl_unswitch): Remove.
> > 	* passes.def (pass_rtl_unswitch): Likewise.
> > 	* loop-init.c (gate_rtl_unswitch): Likewise.
> > 	(rtl_unswitch): Likewise.
> > 	(pass_data_rtl_unswitch): Likewise.
> > 	(pass_rtl_unswitch): Likewise.
> > 	(make_pass_rtl_unswitch): Likewise.
> > 	* rtl.h (reversed_condition): Likewise.
> > 	(compare_and_jump_seq): Likewise.
> > 	* loop-iv.c (reversed_condition): Move here from loop-unswitch.c
> > 	and make static.
> > 	* loop-unroll.c (compare_and_jump_seq): Likewise.
> > 
> > Index: gcc/Makefile.in
> > ===================================================================
> > --- gcc/Makefile.in	(revision 209410)
> > +++ gcc/Makefile.in	(working copy)
> > @@ -1294,7 +1294,6 @@ OBJS = \
> >  	loop-invariant.o \
> >  	loop-iv.o \
> >  	loop-unroll.o \
> > -	loop-unswitch.o \
> >  	lower-subreg.o \
> >  	lra.o \
> >  	lra-assigns.o \
> > Index: gcc/tree-pass.h
> > ===================================================================
> > --- gcc/tree-pass.h	(revision 209410)
> > +++ gcc/tree-pass.h	(working copy)
> > @@ -512,7 +512,6 @@ extern rtl_opt_pass *make_pass_outof_cfg
> >  extern rtl_opt_pass *make_pass_loop2 (gcc::context *ctxt);
> >  extern rtl_opt_pass *make_pass_rtl_loop_init (gcc::context *ctxt);
> >  extern rtl_opt_pass *make_pass_rtl_move_loop_invariants (gcc::context *ctxt);
> > -extern rtl_opt_pass *make_pass_rtl_unswitch (gcc::context *ctxt);
> >  extern rtl_opt_pass *make_pass_rtl_unroll_and_peel_loops (gcc::context *ctxt);
> >  extern rtl_opt_pass *make_pass_rtl_doloop (gcc::context *ctxt);
> >  extern rtl_opt_pass *make_pass_rtl_loop_done (gcc::context *ctxt);
> > Index: gcc/passes.def
> > ===================================================================
> > --- gcc/passes.def	(revision 209410)
> > +++ gcc/passes.def	(working copy)
> > @@ -341,7 +341,6 @@ along with GCC; see the file COPYING3.
> >        PUSH_INSERT_PASSES_WITHIN (pass_loop2)
> >  	  NEXT_PASS (pass_rtl_loop_init);
> >  	  NEXT_PASS (pass_rtl_move_loop_invariants);
> > -	  NEXT_PASS (pass_rtl_unswitch);
> >  	  NEXT_PASS (pass_rtl_unroll_and_peel_loops);
> >  	  NEXT_PASS (pass_rtl_doloop);
> >  	  NEXT_PASS (pass_rtl_loop_done);
> > Index: gcc/loop-init.c
> > ===================================================================
> > --- gcc/loop-init.c	(revision 209410)
> > +++ gcc/loop-init.c	(working copy)
> > @@ -518,61 +518,7 @@ make_pass_rtl_move_loop_invariants (gcc:
> >  }
> >  
> >  \f
> > -/* Loop unswitching for RTL.  */
> > -static bool
> > -gate_rtl_unswitch (void)
> > -{
> > -  return flag_unswitch_loops;
> > -}
> > -
> > -static unsigned int
> > -rtl_unswitch (void)
> > -{
> > -  if (number_of_loops (cfun) > 1)
> > -    unswitch_loops ();
> > -  return 0;
> > -}
> > -
> > -namespace {
> > -
> > -const pass_data pass_data_rtl_unswitch =
> > -{
> > -  RTL_PASS, /* type */
> > -  "loop2_unswitch", /* name */
> > -  OPTGROUP_LOOP, /* optinfo_flags */
> > -  true, /* has_gate */
> > -  true, /* has_execute */
> > -  TV_LOOP_UNSWITCH, /* tv_id */
> > -  0, /* properties_required */
> > -  0, /* properties_provided */
> > -  0, /* properties_destroyed */
> > -  0, /* todo_flags_start */
> > -  TODO_verify_rtl_sharing, /* todo_flags_finish */
> > -};
> > -
> > -class pass_rtl_unswitch : public rtl_opt_pass
> > -{
> > -public:
> > -  pass_rtl_unswitch (gcc::context *ctxt)
> > -    : rtl_opt_pass (pass_data_rtl_unswitch, ctxt)
> > -  {}
> > -
> > -  /* opt_pass methods: */
> > -  bool gate () { return gate_rtl_unswitch (); }
> > -  unsigned int execute () { return rtl_unswitch (); }
> > -
> > -}; // class pass_rtl_unswitch
> > -
> > -} // anon namespace
> > -
> > -rtl_opt_pass *
> > -make_pass_rtl_unswitch (gcc::context *ctxt)
> > -{
> > -  return new pass_rtl_unswitch (ctxt);
> > -}
> > -
> > -\f
> > -/* Loop unswitching for RTL.  */
> > +/* Loop unrolling and peeling for RTL.  */
> >  static bool
> >  gate_rtl_unroll_and_peel_loops (void)
> >  {
> > Index: gcc/loop-iv.c
> > ===================================================================
> > --- gcc/loop-iv.c	(revision 209410)
> > +++ gcc/loop-iv.c	(working copy)
> > @@ -1732,6 +1732,21 @@ canon_condition (rtx cond)
> >    return cond;
> >  }
> >  
> > +/* Reverses CONDition; returns NULL if we cannot.  */
> > +
> > +static rtx
> > +reversed_condition (rtx cond)
> > +{
> > +  enum rtx_code reversed;
> > +  reversed = reversed_comparison_code (cond, NULL);
> > +  if (reversed == UNKNOWN)
> > +    return NULL_RTX;
> > +  else
> > +    return gen_rtx_fmt_ee (reversed,
> > +			   GET_MODE (cond), XEXP (cond, 0),
> > +			   XEXP (cond, 1));
> > +}
> > +
> >  /* Tries to use the fact that COND holds to simplify EXPR.  ALTERED is the
> >     set of altered regs.  */
> >  
> > Index: gcc/loop-unroll.c
> > ===================================================================
> > --- gcc/loop-unroll.c	(revision 209410)
> > +++ gcc/loop-unroll.c	(working copy)
> > @@ -1060,6 +1060,59 @@ split_edge_and_insert (edge e, rtx insns
> >    return bb;
> >  }
> >  
> > +/* Prepare a sequence comparing OP0 with OP1 using COMP and jumping to LABEL if
> > +   true, with probability PROB.  If CINSN is not NULL, it is the insn to copy
> > +   in order to create a jump.  */
> > +
> > +static rtx
> > +compare_and_jump_seq (rtx op0, rtx op1, enum rtx_code comp, rtx label, int prob,
> > +		      rtx cinsn)
> > +{
> > +  rtx seq, jump, cond;
> > +  enum machine_mode mode;
> > +
> > +  mode = GET_MODE (op0);
> > +  if (mode == VOIDmode)
> > +    mode = GET_MODE (op1);
> > +
> > +  start_sequence ();
> > +  if (GET_MODE_CLASS (mode) == MODE_CC)
> > +    {
> > +      /* A hack -- there seems to be no easy generic way how to make a
> > +	 conditional jump from a ccmode comparison.  */
> > +      gcc_assert (cinsn);
> > +      cond = XEXP (SET_SRC (pc_set (cinsn)), 0);
> > +      gcc_assert (GET_CODE (cond) == comp);
> > +      gcc_assert (rtx_equal_p (op0, XEXP (cond, 0)));
> > +      gcc_assert (rtx_equal_p (op1, XEXP (cond, 1)));
> > +      emit_jump_insn (copy_insn (PATTERN (cinsn)));
> > +      jump = get_last_insn ();
> > +      gcc_assert (JUMP_P (jump));
> > +      JUMP_LABEL (jump) = JUMP_LABEL (cinsn);
> > +      LABEL_NUSES (JUMP_LABEL (jump))++;
> > +      redirect_jump (jump, label, 0);
> > +    }
> > +  else
> > +    {
> > +      gcc_assert (!cinsn);
> > +
> > +      op0 = force_operand (op0, NULL_RTX);
> > +      op1 = force_operand (op1, NULL_RTX);
> > +      do_compare_rtx_and_jump (op0, op1, comp, 0,
> > +			       mode, NULL_RTX, NULL_RTX, label, -1);
> > +      jump = get_last_insn ();
> > +      gcc_assert (JUMP_P (jump));
> > +      JUMP_LABEL (jump) = label;
> > +      LABEL_NUSES (label)++;
> > +    }
> > +  add_int_reg_note (jump, REG_BR_PROB, prob);
> > +
> > +  seq = get_insns ();
> > +  end_sequence ();
> > +
> > +  return seq;
> > +}
> > +
> >  /* Unroll LOOP for which we are able to count number of iterations in runtime
> >     LOOP->LPT_DECISION.TIMES times.  The transformation does this (with some
> >     extra care for case n < 0):
> > Index: gcc/rtl.h
> > ===================================================================
> > --- gcc/rtl.h	(revision 209410)
> > +++ gcc/rtl.h	(working copy)
> > @@ -2743,10 +2743,6 @@ extern unsigned int variable_tracking_ma
> >  extern void get_mode_bounds (enum machine_mode, int, enum machine_mode,
> >  			     rtx *, rtx *);
> >  
> > -/* In loop-unswitch.c  */
> > -extern rtx reversed_condition (rtx);
> > -extern rtx compare_and_jump_seq (rtx, rtx, enum rtx_code, rtx, int, rtx);
> > -
> >  /* In loop-iv.c  */
> >  extern rtx canon_condition (rtx);
> >  extern void simplify_using_condition (rtx, rtx *, bitmap);
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][RFC] Remove RTL loop unswitching
  2014-04-23  8:20   ` Richard Biener
@ 2014-04-23 16:32     ` Jan Hubicka
  0 siblings, 0 replies; 8+ messages in thread
From: Jan Hubicka @ 2014-04-23 16:32 UTC (permalink / raw)
  To: Richard Biener; +Cc: Jan Hubicka, gcc-patches

> On Sun, 20 Apr 2014, Jan Hubicka wrote:
> 
> > > 
> > > This removes RTL loop unswitching (see last years discussion about
> > > compile-time issues of that pass).  RTL loop unswitching is
> > > enabled together with GIMPLE loop unswitching at -O3 and by
> > > -floop-unswitch.  It's clearly the wrong place to do high-level
> > > loop transforms these days, and the cost of maintainance doesn't
> > > outweight the questionable benefit.
> > > 
> > > Thus the following patch removes it.
> > > 
> > > Bootstrap / regtest pending on x86_64-unknown-linux-gnu (I hope
> > > for testsuite fallout).
> > > 
> > > Any objections?
> > 
> > Not really, I am all for moving more of loop stuff to trees.
> > Did you performed some benchmarks? (I remember I did in 2012
> > but completely forgot the outcome).
> 
> I did that last year and it showed no difference in SPEC 2k6.
> 
> When bootstrapping with -O3 and a gcc_unreachable () in the
> RTL unswitching path you get some ICEs there but they are
> due to different "effective" --param max-unswitch-insns that
> is on GIMPLE applied to tree_num_loop_insns () and on RTL
> to num_loop_insns ().

Yep, I remember seeing some interesting special cases where RTL analyzis
did catch on invariants but tree didn't, but nothing important.
> 
> I'll go forward with the patch today.
> 
> > On related note, shall I try to update the following?
> > http://gcc.gnu.org/ml/gcc-patches/2013-11/msg02270.html
> 
> Yeah.

Will do,
Honza
> 
> Thanks,
> Richard.
> 
> > Honza
> > > 
> > > Thanks,
> > > Richard.
> > > 
> > > 2014-04-15  Richard Biener  <rguenther@suse.de>
> > > 
> > > 	* Makefile.in (OBJS): Remove loop-unswitch.o.
> > > 	* loop-unswitch.c: Delete.
> > > 	* tree-pass.h (make_pass_rtl_unswitch): Remove.
> > > 	* passes.def (pass_rtl_unswitch): Likewise.
> > > 	* loop-init.c (gate_rtl_unswitch): Likewise.
> > > 	(rtl_unswitch): Likewise.
> > > 	(pass_data_rtl_unswitch): Likewise.
> > > 	(pass_rtl_unswitch): Likewise.
> > > 	(make_pass_rtl_unswitch): Likewise.
> > > 	* rtl.h (reversed_condition): Likewise.
> > > 	(compare_and_jump_seq): Likewise.
> > > 	* loop-iv.c (reversed_condition): Move here from loop-unswitch.c
> > > 	and make static.
> > > 	* loop-unroll.c (compare_and_jump_seq): Likewise.
> > > 
> > > Index: gcc/Makefile.in
> > > ===================================================================
> > > --- gcc/Makefile.in	(revision 209410)
> > > +++ gcc/Makefile.in	(working copy)
> > > @@ -1294,7 +1294,6 @@ OBJS = \
> > >  	loop-invariant.o \
> > >  	loop-iv.o \
> > >  	loop-unroll.o \
> > > -	loop-unswitch.o \
> > >  	lower-subreg.o \
> > >  	lra.o \
> > >  	lra-assigns.o \
> > > Index: gcc/tree-pass.h
> > > ===================================================================
> > > --- gcc/tree-pass.h	(revision 209410)
> > > +++ gcc/tree-pass.h	(working copy)
> > > @@ -512,7 +512,6 @@ extern rtl_opt_pass *make_pass_outof_cfg
> > >  extern rtl_opt_pass *make_pass_loop2 (gcc::context *ctxt);
> > >  extern rtl_opt_pass *make_pass_rtl_loop_init (gcc::context *ctxt);
> > >  extern rtl_opt_pass *make_pass_rtl_move_loop_invariants (gcc::context *ctxt);
> > > -extern rtl_opt_pass *make_pass_rtl_unswitch (gcc::context *ctxt);
> > >  extern rtl_opt_pass *make_pass_rtl_unroll_and_peel_loops (gcc::context *ctxt);
> > >  extern rtl_opt_pass *make_pass_rtl_doloop (gcc::context *ctxt);
> > >  extern rtl_opt_pass *make_pass_rtl_loop_done (gcc::context *ctxt);
> > > Index: gcc/passes.def
> > > ===================================================================
> > > --- gcc/passes.def	(revision 209410)
> > > +++ gcc/passes.def	(working copy)
> > > @@ -341,7 +341,6 @@ along with GCC; see the file COPYING3.
> > >        PUSH_INSERT_PASSES_WITHIN (pass_loop2)
> > >  	  NEXT_PASS (pass_rtl_loop_init);
> > >  	  NEXT_PASS (pass_rtl_move_loop_invariants);
> > > -	  NEXT_PASS (pass_rtl_unswitch);
> > >  	  NEXT_PASS (pass_rtl_unroll_and_peel_loops);
> > >  	  NEXT_PASS (pass_rtl_doloop);
> > >  	  NEXT_PASS (pass_rtl_loop_done);
> > > Index: gcc/loop-init.c
> > > ===================================================================
> > > --- gcc/loop-init.c	(revision 209410)
> > > +++ gcc/loop-init.c	(working copy)
> > > @@ -518,61 +518,7 @@ make_pass_rtl_move_loop_invariants (gcc:
> > >  }
> > >  
> > >  \f
> > > -/* Loop unswitching for RTL.  */
> > > -static bool
> > > -gate_rtl_unswitch (void)
> > > -{
> > > -  return flag_unswitch_loops;
> > > -}
> > > -
> > > -static unsigned int
> > > -rtl_unswitch (void)
> > > -{
> > > -  if (number_of_loops (cfun) > 1)
> > > -    unswitch_loops ();
> > > -  return 0;
> > > -}
> > > -
> > > -namespace {
> > > -
> > > -const pass_data pass_data_rtl_unswitch =
> > > -{
> > > -  RTL_PASS, /* type */
> > > -  "loop2_unswitch", /* name */
> > > -  OPTGROUP_LOOP, /* optinfo_flags */
> > > -  true, /* has_gate */
> > > -  true, /* has_execute */
> > > -  TV_LOOP_UNSWITCH, /* tv_id */
> > > -  0, /* properties_required */
> > > -  0, /* properties_provided */
> > > -  0, /* properties_destroyed */
> > > -  0, /* todo_flags_start */
> > > -  TODO_verify_rtl_sharing, /* todo_flags_finish */
> > > -};
> > > -
> > > -class pass_rtl_unswitch : public rtl_opt_pass
> > > -{
> > > -public:
> > > -  pass_rtl_unswitch (gcc::context *ctxt)
> > > -    : rtl_opt_pass (pass_data_rtl_unswitch, ctxt)
> > > -  {}
> > > -
> > > -  /* opt_pass methods: */
> > > -  bool gate () { return gate_rtl_unswitch (); }
> > > -  unsigned int execute () { return rtl_unswitch (); }
> > > -
> > > -}; // class pass_rtl_unswitch
> > > -
> > > -} // anon namespace
> > > -
> > > -rtl_opt_pass *
> > > -make_pass_rtl_unswitch (gcc::context *ctxt)
> > > -{
> > > -  return new pass_rtl_unswitch (ctxt);
> > > -}
> > > -
> > > -\f
> > > -/* Loop unswitching for RTL.  */
> > > +/* Loop unrolling and peeling for RTL.  */
> > >  static bool
> > >  gate_rtl_unroll_and_peel_loops (void)
> > >  {
> > > Index: gcc/loop-iv.c
> > > ===================================================================
> > > --- gcc/loop-iv.c	(revision 209410)
> > > +++ gcc/loop-iv.c	(working copy)
> > > @@ -1732,6 +1732,21 @@ canon_condition (rtx cond)
> > >    return cond;
> > >  }
> > >  
> > > +/* Reverses CONDition; returns NULL if we cannot.  */
> > > +
> > > +static rtx
> > > +reversed_condition (rtx cond)
> > > +{
> > > +  enum rtx_code reversed;
> > > +  reversed = reversed_comparison_code (cond, NULL);
> > > +  if (reversed == UNKNOWN)
> > > +    return NULL_RTX;
> > > +  else
> > > +    return gen_rtx_fmt_ee (reversed,
> > > +			   GET_MODE (cond), XEXP (cond, 0),
> > > +			   XEXP (cond, 1));
> > > +}
> > > +
> > >  /* Tries to use the fact that COND holds to simplify EXPR.  ALTERED is the
> > >     set of altered regs.  */
> > >  
> > > Index: gcc/loop-unroll.c
> > > ===================================================================
> > > --- gcc/loop-unroll.c	(revision 209410)
> > > +++ gcc/loop-unroll.c	(working copy)
> > > @@ -1060,6 +1060,59 @@ split_edge_and_insert (edge e, rtx insns
> > >    return bb;
> > >  }
> > >  
> > > +/* Prepare a sequence comparing OP0 with OP1 using COMP and jumping to LABEL if
> > > +   true, with probability PROB.  If CINSN is not NULL, it is the insn to copy
> > > +   in order to create a jump.  */
> > > +
> > > +static rtx
> > > +compare_and_jump_seq (rtx op0, rtx op1, enum rtx_code comp, rtx label, int prob,
> > > +		      rtx cinsn)
> > > +{
> > > +  rtx seq, jump, cond;
> > > +  enum machine_mode mode;
> > > +
> > > +  mode = GET_MODE (op0);
> > > +  if (mode == VOIDmode)
> > > +    mode = GET_MODE (op1);
> > > +
> > > +  start_sequence ();
> > > +  if (GET_MODE_CLASS (mode) == MODE_CC)
> > > +    {
> > > +      /* A hack -- there seems to be no easy generic way how to make a
> > > +	 conditional jump from a ccmode comparison.  */
> > > +      gcc_assert (cinsn);
> > > +      cond = XEXP (SET_SRC (pc_set (cinsn)), 0);
> > > +      gcc_assert (GET_CODE (cond) == comp);
> > > +      gcc_assert (rtx_equal_p (op0, XEXP (cond, 0)));
> > > +      gcc_assert (rtx_equal_p (op1, XEXP (cond, 1)));
> > > +      emit_jump_insn (copy_insn (PATTERN (cinsn)));
> > > +      jump = get_last_insn ();
> > > +      gcc_assert (JUMP_P (jump));
> > > +      JUMP_LABEL (jump) = JUMP_LABEL (cinsn);
> > > +      LABEL_NUSES (JUMP_LABEL (jump))++;
> > > +      redirect_jump (jump, label, 0);
> > > +    }
> > > +  else
> > > +    {
> > > +      gcc_assert (!cinsn);
> > > +
> > > +      op0 = force_operand (op0, NULL_RTX);
> > > +      op1 = force_operand (op1, NULL_RTX);
> > > +      do_compare_rtx_and_jump (op0, op1, comp, 0,
> > > +			       mode, NULL_RTX, NULL_RTX, label, -1);
> > > +      jump = get_last_insn ();
> > > +      gcc_assert (JUMP_P (jump));
> > > +      JUMP_LABEL (jump) = label;
> > > +      LABEL_NUSES (label)++;
> > > +    }
> > > +  add_int_reg_note (jump, REG_BR_PROB, prob);
> > > +
> > > +  seq = get_insns ();
> > > +  end_sequence ();
> > > +
> > > +  return seq;
> > > +}
> > > +
> > >  /* Unroll LOOP for which we are able to count number of iterations in runtime
> > >     LOOP->LPT_DECISION.TIMES times.  The transformation does this (with some
> > >     extra care for case n < 0):
> > > Index: gcc/rtl.h
> > > ===================================================================
> > > --- gcc/rtl.h	(revision 209410)
> > > +++ gcc/rtl.h	(working copy)
> > > @@ -2743,10 +2743,6 @@ extern unsigned int variable_tracking_ma
> > >  extern void get_mode_bounds (enum machine_mode, int, enum machine_mode,
> > >  			     rtx *, rtx *);
> > >  
> > > -/* In loop-unswitch.c  */
> > > -extern rtx reversed_condition (rtx);
> > > -extern rtx compare_and_jump_seq (rtx, rtx, enum rtx_code, rtx, int, rtx);
> > > -
> > >  /* In loop-iv.c  */
> > >  extern rtx canon_condition (rtx);
> > >  extern void simplify_using_condition (rtx, rtx *, bitmap);
> > 
> > 
> 
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE / SUSE Labs
> SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
> GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][RFC] Remove RTL loop unswitching
  2014-04-15  9:28 [PATCH][RFC] Remove RTL loop unswitching Richard Biener
  2014-04-15 12:58 ` Richard Biener
  2014-04-20  7:09 ` Jan Hubicka
@ 2014-05-07 10:34 ` Thomas Schwinge
  2014-05-07 12:45 ` Thomas Schwinge
  3 siblings, 0 replies; 8+ messages in thread
From: Thomas Schwinge @ 2014-05-07 10:34 UTC (permalink / raw)
  To: gcc-patches; +Cc: Richard Biener

[-- Attachment #1: Type: text/plain, Size: 1969 bytes --]

Hi!

On Tue, 15 Apr 2014 11:26:29 +0200 (CEST), Richard Biener <rguenther@suse.de> wrote:
> This removes RTL loop unswitching

> 2014-04-15  Richard Biener  <rguenther@suse.de>
> 
> 	* Makefile.in (OBJS): Remove loop-unswitch.o.
> 	* loop-unswitch.c: Delete.
> 	* tree-pass.h (make_pass_rtl_unswitch): Remove.
> 	* passes.def (pass_rtl_unswitch): Likewise.
> 	* loop-init.c (gate_rtl_unswitch): Likewise.
> 	(rtl_unswitch): Likewise.
> 	(pass_data_rtl_unswitch): Likewise.
> 	(pass_rtl_unswitch): Likewise.
> 	(make_pass_rtl_unswitch): Likewise.
> 	* rtl.h (reversed_condition): Likewise.
> 	(compare_and_jump_seq): Likewise.
> 	* loop-iv.c (reversed_condition): Move here from loop-unswitch.c
> 	and make static.
> 	* loop-unroll.c (compare_and_jump_seq): Likewise.

After checking with Richard on IRC, I applied the following in r210150:

commit 81283dac62a91d2fbdf154fe51e9f84e0b1db816
Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Wed May 7 10:31:26 2014 +0000

    Really delete gcc/loop-unswitch.c.
    
    	gcc/
    	* loop-unswitch.c: Delete.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@210150 138bc75d-0d04-0410-961f-82ee72b054a4

diff --git gcc/ChangeLog gcc/ChangeLog
index d5e6a0a..e5033a0 100644
--- gcc/ChangeLog
+++ gcc/ChangeLog
@@ -1,3 +1,7 @@
+2014-05-07  Thomas Schwinge  <thomas@codesourcery.com>
+
+	* loop-unswitch.c: Delete.
+
 2014-05-07  Richard Biener  <rguenther@suse.de>
 
 	* config.gcc: Always set need_64bit_hwint to yes.
@@ -2294,7 +2298,6 @@
 2014-04-23  Richard Biener  <rguenther@suse.de>
 
 	* Makefile.in (OBJS): Remove loop-unswitch.o.
-	* loop-unswitch.c: Delete.
 	* tree-pass.h (make_pass_rtl_unswitch): Remove.
 	* passes.def (pass_rtl_unswitch): Likewise.
 	* loop-init.c (gate_rtl_unswitch): Likewise.
diff --git gcc/loop-unswitch.c gcc/loop-unswitch.c
deleted file mode 100644
index fff0fd1..0000000


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 472 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][RFC] Remove RTL loop unswitching
  2014-04-15  9:28 [PATCH][RFC] Remove RTL loop unswitching Richard Biener
                   ` (2 preceding siblings ...)
  2014-05-07 10:34 ` Thomas Schwinge
@ 2014-05-07 12:45 ` Thomas Schwinge
  2014-05-07 12:47   ` Richard Biener
  3 siblings, 1 reply; 8+ messages in thread
From: Thomas Schwinge @ 2014-05-07 12:45 UTC (permalink / raw)
  To: Richard Biener, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 3814 bytes --]

Hi!

On Tue, 15 Apr 2014 11:26:29 +0200 (CEST), Richard Biener <rguenther@suse.de> wrote:
> This removes RTL loop unswitching

> 2014-04-15  Richard Biener  <rguenther@suse.de>
> 
> 	* Makefile.in (OBJS): Remove loop-unswitch.o.
> 	* loop-unswitch.c: Delete.
> 	* tree-pass.h (make_pass_rtl_unswitch): Remove.
> 	* passes.def (pass_rtl_unswitch): Likewise.
> 	* loop-init.c (gate_rtl_unswitch): Likewise.
> 	(rtl_unswitch): Likewise.
> 	(pass_data_rtl_unswitch): Likewise.
> 	(pass_rtl_unswitch): Likewise.
> 	(make_pass_rtl_unswitch): Likewise.
> 	* rtl.h (reversed_condition): Likewise.
> 	(compare_and_jump_seq): Likewise.
> 	* loop-iv.c (reversed_condition): Move here from loop-unswitch.c
> 	and make static.
> 	* loop-unroll.c (compare_and_jump_seq): Likewise.

I found some more; OK to commit?  Is a non-bootstrap build enough for
this, or is a full bootstrap build and test needed?

commit 8a703b1e7adc6001f665a12f93601382e3eea806
Author: Thomas Schwinge <thomas@codesourcery.com>
Date:   Wed May 7 13:01:47 2014 +0200

    More gcc/loop-unswitch.c cleanup.
    
    	gcc/
    	* cfgloop.h (unswitch_loops): Remove.
    	* doc/passes.texi: Remove references to loop-unswitch.c
    	* timevar.def (TV_LOOP_UNSWITCH): Remove.

diff --git gcc/cfgloop.h gcc/cfgloop.h
index ab8b809..62a656a 100644
--- gcc/cfgloop.h
+++ gcc/cfgloop.h
@@ -711,8 +711,6 @@ extern void loop_optimizer_init (unsigned);
 extern void loop_optimizer_finalize (void);
 
 /* Optimization passes.  */
-extern void unswitch_loops (void);
-
 enum
 {
   UAP_PEEL = 1,		/* Enables loop peeling.  */
diff --git gcc/doc/passes.texi gcc/doc/passes.texi
index 2727b2c..fb064db 100644
--- gcc/doc/passes.texi
+++ gcc/doc/passes.texi
@@ -474,10 +474,7 @@ merging and induction variable elimination.  The pass is implemented in
 Loop unswitching.  This pass moves the conditional jumps that are invariant
 out of the loops.  To achieve this, a duplicate of the loop is created for
 each possible outcome of conditional jump(s).  The pass is implemented in
-@file{tree-ssa-loop-unswitch.c}.  This pass should eventually replace the
-RTL level loop unswitching in @file{loop-unswitch.c}, but currently
-the RTL level pass is not completely redundant yet due to deficiencies
-in tree level alias analysis.
+@file{tree-ssa-loop-unswitch.c}.
 
 The optimizations also use various utility functions contained in
 @file{tree-ssa-loop-manip.c}, @file{cfgloop.c}, @file{cfgloopanal.c} and
@@ -793,8 +790,8 @@ The source files @file{cfgloopanal.c} and @file{cfgloopmanip.c} contain
 generic loop analysis and manipulation code.  Initialization and finalization
 of loop structures is handled by @file{loop-init.c}.
 A loop invariant motion pass is implemented in @file{loop-invariant.c}.
-Basic block level optimizations---unrolling, peeling and unswitching loops---
-are implemented in @file{loop-unswitch.c} and @file{loop-unroll.c}.
+Basic block level optimizations---unrolling, and peeling loops---
+are implemented in @file{loop-unroll.c}.
 Replacing of the exit condition of loops by special machine-dependent
 instructions is handled by @file{loop-doloop.c}.
 
diff --git gcc/timevar.def gcc/timevar.def
index 9faf98b..2db1943 100644
--- gcc/timevar.def
+++ gcc/timevar.def
@@ -207,7 +207,6 @@ DEFTIMEVAR (TV_DSE2                  , "dead store elim2")
 DEFTIMEVAR (TV_LOOP                  , "loop analysis")
 DEFTIMEVAR (TV_LOOP_INIT	     , "loop init")
 DEFTIMEVAR (TV_LOOP_MOVE_INVARIANTS  , "loop invariant motion")
-DEFTIMEVAR (TV_LOOP_UNSWITCH         , "loop unswitching")
 DEFTIMEVAR (TV_LOOP_UNROLL           , "loop unrolling")
 DEFTIMEVAR (TV_LOOP_DOLOOP           , "loop doloop")
 DEFTIMEVAR (TV_LOOP_FINI	     , "loop fini")


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 472 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][RFC] Remove RTL loop unswitching
  2014-05-07 12:45 ` Thomas Schwinge
@ 2014-05-07 12:47   ` Richard Biener
  0 siblings, 0 replies; 8+ messages in thread
From: Richard Biener @ 2014-05-07 12:47 UTC (permalink / raw)
  To: Thomas Schwinge; +Cc: gcc-patches

[-- Attachment #1: Type: TEXT/PLAIN, Size: 4164 bytes --]

On Wed, 7 May 2014, Thomas Schwinge wrote:

> Hi!
> 
> On Tue, 15 Apr 2014 11:26:29 +0200 (CEST), Richard Biener <rguenther@suse.de> wrote:
> > This removes RTL loop unswitching
> 
> > 2014-04-15  Richard Biener  <rguenther@suse.de>
> > 
> > 	* Makefile.in (OBJS): Remove loop-unswitch.o.
> > 	* loop-unswitch.c: Delete.
> > 	* tree-pass.h (make_pass_rtl_unswitch): Remove.
> > 	* passes.def (pass_rtl_unswitch): Likewise.
> > 	* loop-init.c (gate_rtl_unswitch): Likewise.
> > 	(rtl_unswitch): Likewise.
> > 	(pass_data_rtl_unswitch): Likewise.
> > 	(pass_rtl_unswitch): Likewise.
> > 	(make_pass_rtl_unswitch): Likewise.
> > 	* rtl.h (reversed_condition): Likewise.
> > 	(compare_and_jump_seq): Likewise.
> > 	* loop-iv.c (reversed_condition): Move here from loop-unswitch.c
> > 	and make static.
> > 	* loop-unroll.c (compare_and_jump_seq): Likewise.
> 
> I found some more; OK to commit?  Is a non-bootstrap build enough for
> this, or is a full bootstrap build and test needed?

That's enough.

Ok.

Thanks,
Richard.

> commit 8a703b1e7adc6001f665a12f93601382e3eea806
> Author: Thomas Schwinge <thomas@codesourcery.com>
> Date:   Wed May 7 13:01:47 2014 +0200
> 
>     More gcc/loop-unswitch.c cleanup.
>     
>     	gcc/
>     	* cfgloop.h (unswitch_loops): Remove.
>     	* doc/passes.texi: Remove references to loop-unswitch.c
>     	* timevar.def (TV_LOOP_UNSWITCH): Remove.
> 
> diff --git gcc/cfgloop.h gcc/cfgloop.h
> index ab8b809..62a656a 100644
> --- gcc/cfgloop.h
> +++ gcc/cfgloop.h
> @@ -711,8 +711,6 @@ extern void loop_optimizer_init (unsigned);
>  extern void loop_optimizer_finalize (void);
>  
>  /* Optimization passes.  */
> -extern void unswitch_loops (void);
> -
>  enum
>  {
>    UAP_PEEL = 1,		/* Enables loop peeling.  */
> diff --git gcc/doc/passes.texi gcc/doc/passes.texi
> index 2727b2c..fb064db 100644
> --- gcc/doc/passes.texi
> +++ gcc/doc/passes.texi
> @@ -474,10 +474,7 @@ merging and induction variable elimination.  The pass is implemented in
>  Loop unswitching.  This pass moves the conditional jumps that are invariant
>  out of the loops.  To achieve this, a duplicate of the loop is created for
>  each possible outcome of conditional jump(s).  The pass is implemented in
> -@file{tree-ssa-loop-unswitch.c}.  This pass should eventually replace the
> -RTL level loop unswitching in @file{loop-unswitch.c}, but currently
> -the RTL level pass is not completely redundant yet due to deficiencies
> -in tree level alias analysis.
> +@file{tree-ssa-loop-unswitch.c}.
>  
>  The optimizations also use various utility functions contained in
>  @file{tree-ssa-loop-manip.c}, @file{cfgloop.c}, @file{cfgloopanal.c} and
> @@ -793,8 +790,8 @@ The source files @file{cfgloopanal.c} and @file{cfgloopmanip.c} contain
>  generic loop analysis and manipulation code.  Initialization and finalization
>  of loop structures is handled by @file{loop-init.c}.
>  A loop invariant motion pass is implemented in @file{loop-invariant.c}.
> -Basic block level optimizations---unrolling, peeling and unswitching loops---
> -are implemented in @file{loop-unswitch.c} and @file{loop-unroll.c}.
> +Basic block level optimizations---unrolling, and peeling loops---
> +are implemented in @file{loop-unroll.c}.
>  Replacing of the exit condition of loops by special machine-dependent
>  instructions is handled by @file{loop-doloop.c}.
>  
> diff --git gcc/timevar.def gcc/timevar.def
> index 9faf98b..2db1943 100644
> --- gcc/timevar.def
> +++ gcc/timevar.def
> @@ -207,7 +207,6 @@ DEFTIMEVAR (TV_DSE2                  , "dead store elim2")
>  DEFTIMEVAR (TV_LOOP                  , "loop analysis")
>  DEFTIMEVAR (TV_LOOP_INIT	     , "loop init")
>  DEFTIMEVAR (TV_LOOP_MOVE_INVARIANTS  , "loop invariant motion")
> -DEFTIMEVAR (TV_LOOP_UNSWITCH         , "loop unswitching")
>  DEFTIMEVAR (TV_LOOP_UNROLL           , "loop unrolling")
>  DEFTIMEVAR (TV_LOOP_DOLOOP           , "loop doloop")
>  DEFTIMEVAR (TV_LOOP_FINI	     , "loop fini")
> 
> 
> Grüße,
>  Thomas
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-05-07 12:47 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-15  9:28 [PATCH][RFC] Remove RTL loop unswitching Richard Biener
2014-04-15 12:58 ` Richard Biener
2014-04-20  7:09 ` Jan Hubicka
2014-04-23  8:20   ` Richard Biener
2014-04-23 16:32     ` Jan Hubicka
2014-05-07 10:34 ` Thomas Schwinge
2014-05-07 12:45 ` Thomas Schwinge
2014-05-07 12:47   ` Richard Biener

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).