[PATCH] Do not use a REG_EQUAL value for gcse/cprop if SRC is a REG

["Steven Bosscher" <stevenb dot gcc at gmail dot com>]

Hi,

If PRE GCSE iterates, you can set --param max-gcse-passes to infinite,
and gcse would still not terminate.  I reported this behavior already
in 2004 (see PR16967) but "normal" users of gcc don't play with
--param max-gcse-passes so it's not a very serious bug.

There are two reasons for this: 1) CPROP undoes some transformations
that PRE does, and PRE then, of course, applies the same
transformation again in the next iteration; and 2) CPROP always takes
the last SET of a value in a basic block, which sometimes has a dead
SET_DEST which isn't eliminated because there is no DCE pass in the
gcse_main() loop.

I think --param max-gcse-passes ought to go away. One reason is that
it's almost not used and therefore almost untested (it doesn't broken
at -Os, for example, and always has been ever since the hoisting code
was introduced).  Another reason is that the most important reason to
do multiple passes has gone away with Paolo Bonzini's patch to perform
PRE GCSE on REG_EQUAL notes.  That was a brilliant idea, because it
allows gcse.c to PRE things like addresses that require multiple insns
to construct.  So, no need to do two PRE passes to move the HIGH and
LOW parts of an address load.  That always was the most important
reason to set --param max-gcse-passes to 2 or 3.

That was a long intro...  So what does all this have to do with
$SUBJECT?  Well...

Unfortunately, it turns out that Paolo's patch has an unexpected side
effect.  I tested --param max-gcse-passes on powerpc.  On the
preprocessed source of cc1 on powerpc-unknown-elf, the first pass
performs 81652 substs and 45263 inserts, and the second pass does 9299
substs and 9851 inserts.  I got to these numbers by adding an extra
pass_gcse in passes.c, and by setting setting DF_LR_RUN_DCE before
df_analyze() in gcse_main(), to avoid the "gcse picks up dead insns"
problem ;-)

Many of the subst and inserts in the second pass are for cases where
the value of a REG_EQUAL note is recorded instead of the SET_SRC of an
insn. And in many cases, the SET_SRC is a REG and the insn was created
in the first GCSE PRE pass.  Basically the second iteration performs
PRE on the same expression as the first.  This introduces unnecessary
moves (that are not always eliminated later on) and a large amount of
of dead code that delete_trivially_dead_insns() sometimes cannot
remove (so the dead code stays until the first rtl_dse pass).
Furthermore, because CPROP unconditionally prefers the REG_EQUAL
constant on reg-reg moves, we miss copy propagation opportunities,
which again introduces more moves that GCC will not always clean up
later on.

With the attached patch, the numbers for the first pass are 81652
substs and 45207 inserts (i.e. practically unchanged), and the second
pass does 2962 inserts and 6879. There are ~3300 fewer instructions in
the final assembler output, mostly due to fewer reg-reg moves (942
fewer mr insns) and fewer spills (711 fewer lwz insns, and 394 fewer
stw insns).  I have found two cases where we missed a PRE opportunity
due to the patch, and several dozen cases where PRE's work wasn't
undone by CPROP2 anymore.  Overall the patch looks like a win to me.

Bootstrapped&tested on powerpc-linux-gnu (32 and 64 bit).  OK for trunk?

Gr.
Steven

	* gcse.c (hash_scan_set): Do not pick up a REG_EQUAL value if
	SRC is a REG.

Index: gcse.c
===================================================================
*** gcse.c	(revision 135818)
--- gcse.c	(working copy)
*************** hash_scan_set (rtx pat, rtx insn, struct
*** 1692,1703 ****
        unsigned int regno = REGNO (dest);
        rtx tmp;
  
!       /* See if a REG_NOTE shows this equivalent to a simpler expression.
  	 This allows us to do a single GCSE pass and still eliminate
  	 redundant constants, addresses or other expressions that are
! 	 constructed with multiple instructions.  */
        note = find_reg_equal_equiv_note (insn);
        if (note != 0
  	  && (table->set_p
  	      ? gcse_constant_p (XEXP (note, 0))
  	      : want_to_gcse_p (XEXP (note, 0))))
--- 1692,1716 ----
        unsigned int regno = REGNO (dest);
        rtx tmp;
  
!       /* See if a REG_EQUAL note shows this equivalent to a simpler expression.
! 
  	 This allows us to do a single GCSE pass and still eliminate
  	 redundant constants, addresses or other expressions that are
! 	 constructed with multiple instructions.
! 
! 	 However, keep the original SRC if INSN is a simple reg-reg move.  In
! 	 In this case, there will almost always be a REG_EQUAL note on the
! 	 insn that sets SRC.  By recording the REG_EQUAL value here as SRC
! 	 for INSN, we miss copy propagation opportunities and we perform the
! 	 same PRE GCSE operation repeatedly on the same REG_EQUAL value if we
! 	 do more than one PRE GCSE pass.
! 
! 	 Note that this does not impede profitale constant propagations.  We
! 	 "look through" reg-reg sets in lookup_avail_set.  */
        note = find_reg_equal_equiv_note (insn);
        if (note != 0
+ 	  && REG_NOTE_KIND (note) == REG_EQUAL
+ 	  && !REG_P (src)
  	  && (table->set_p
  	      ? gcse_constant_p (XEXP (note, 0))
  	      : want_to_gcse_p (XEXP (note, 0))))