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

["Richard Guenther" <richard dot guenther at gmail dot com>]

On Sun, May 25, 2008 at 10:57 AM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> 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?

This is ok.

Thanks,
Richard.