Re: Pre-inline optimization

[Jan Hubicka <hubicka at ucw dot cz>]

> 
> I wonder how you came to the current set of passes and their ordering.  I

I just copied the passes from initial scalar cleanups and removed those
I want to deal with separately.
> also wonder if we can adjust some of the initial scalar cleanup passes now.
> You have
> 
> +   NEXT_PASS (pass_rename_ssa_copies);
> +   NEXT_PASS (pass_ccp);
> +   NEXT_PASS (pass_forwprop);
> +   NEXT_PASS (pass_copy_prop);
> +   NEXT_PASS (pass_merge_phi);
> +   NEXT_PASS (pass_copy_prop);
> +   NEXT_PASS (pass_dce);
> +   NEXT_PASS (pass_tail_recursion);
> 
> and in initial scalar cleanups we have
> 
>  /* Initial scalar cleanups.  */
>  NEXT_PASS (pass_ccp);
>  NEXT_PASS (pass_fre);
>  NEXT_PASS (pass_dce);
>  NEXT_PASS (pass_forwprop);
>  NEXT_PASS (pass_copy_prop);
>  NEXT_PASS (pass_merge_phi);
>  NEXT_PASS (pass_vrp);
>  NEXT_PASS (pass_dce);
>  NEXT_PASS (pass_dominator);
> 
> I wonder why you need two copyprop passes added?  I would also

Huh, this is omision.  I used to have fre/dom there, but I decided to
deal with it later. I've dropped the second copyprop from my local
copy of pass and it don't seem to change anything.

> suggest changing the ccp and copy_prop passes in the initial
> scalar cleanup section to the store_ccp and store_copy_prop
> variants and re-ordering the forwprop pass there until after the
> first dominator pass and remove the first invocation of dce.
> 
> What kind of propagation does the inliner now do?  I remember
> something about const and copy propagation?

It is simply renaming SSA names assigned to arguments by the arguments
instead of emitting copy instruction like old inliner did.  This means
that arguments are constant and copy propagated.

Still the set of initial scalar cleanups is definitly not replaced by
the pre-inline optimizations simply because pre-inline don't see
aliasing and also inlining introduces more opitimization opurtunities
except for the propagation of argument mentioned above.
(Inliner is also good source of dead code.)

I was also considering that if the early cleanups was re-run on every
functions significantly modified by IPA, we can drop some of re-running
of passes in our later queue more easilly, but didn't tested it yet.
> 
> Do we need the second tail recursion pass?

Yes - inlining mutually recursive functions introduce tail recursion and
also post-inline tail recursion can use aliasing and thus is slightly
stronger, so it is not dificult to construct testcases where only second
tail recursion match.
Since the pass don't even walk whole SSA graph, the cost is about 0.
> 
> I know it's always easy to add passes and very hard to argue we can
> remove/re-order some - but the point of adding some should be
> the easiest time to argue to remove/re-order some others ;)

Well, I hopes to deal with this in little bit incremental fashion.
I.e. first start with the cheap passes, then see if some of bigger
hammers make sense pre-inline and try to rationalize the post-inline
queue.  THere are unfurtunately a lot more unknowns that I would feel
confortable

I deifnitly can SPEC benchmark the changes you suggest however.
> 
> Also maybe PR23346 is fixed by your patch?

My tree seems somewhere in between:

f:
        xorl    %eax, %eax
        movl    a, %edx
        cmpl    $0, 4(%esp)
        movl    b, %ecx
        jne     .L4
        movl    %edx, %eax
        sarl    $31, %edx
        idivl   %ecx
.L4:
        rep ; ret

Honza
> 
> Thanks,
> Richard.