> 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  <rguent...@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:
> > >  }
> > >  
> > >  
> > > -/* 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);
> > > -}
> > > -
> > > -
> > > -/* 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 <rguent...@suse.de>
> SUSE / SUSE Labs
> SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
> GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer

Reply via email to