| Message ID | CA+=Sn1n9JqQrOusLsvXmD1fHy5E0jYasYZSzAbsL6kEMi_12NQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Jan 23, 2012 at 6:33 AM, Andrew Pinski <andrew.pinski@caviumnetworks.com> wrote: > Hi, > The problem here is not really jump threading getting in the way of > PHI-OPT rather there is no cleanup of the IR after jump thread but > before phi-opt. > This patch adds a pass_phi_only_cprop right after the first vrp and > pass_merge_phi right before the first and last phiopts. > > Since VRP does jump threading just like DOM does, we need a > pass_phi_only_cprop right after it. And the merge PHI pass should be > done so that going into PHI-OPT we have a CFG which there is only one > PHI for a case like: > bb0: > if (a) > goto L1; > bb3: > if (b) goto L2; > L1: > x = PHI <b(3), c(0)> > L2: > x= PHI<x(L1), d(3)> > > PHI-OPT does not handle PHIs like this that well. > > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. The testcase already works for me on trunk, without your patch. You are adding three passes - one of it phi_only_cprop which I'd rather remove ... I suspect VRP simply exposes copy propagation opportunities, and that's expected. mergephi before phiopt makes sense, but then we should move the first mergephi instead of inserting another one. Richard. > Thanks, > Andrew Pinski > > ChangeLog: > * passes.c (init_optimization_passes): Add pass_phi_only_cprop after > the first vpr pass. > Add pass_merge_phi before the first and last phiopt passes. > > testsuite/ChangeLog: > * gcc.dg/tree-ssa/phi-opt-7.c: New testcase.
Index: testsuite/gcc.dg/tree-ssa/phi-opt-7.c =================================================================== --- testsuite/gcc.dg/tree-ssa/phi-opt-7.c (revision 0) +++ testsuite/gcc.dg/tree-ssa/phi-opt-7.c (revision 0) @@ -0,0 +1,25 @@ +/* { dg-do compile } */ +/* { dg-options "-O -fdump-tree-phiopt1" } */ + + +int minmax_correct(int a) +{ + if (a > 32767) a = 32767; + else if (a < -32768) a = -32768; + + return a; +} + +int minmax_wrong(int a) +{ + if (a > 32767) a = 32767; + if (a < -32768) a = -32768; + + return a; +} + +/* { dg-final { scan-tree-dump-not "if" "phiopt1" } } */ +/* { dg-final { scan-tree-dump-times "MAX_EXPR" 2 "phiopt1" } } */ +/* { dg-final { scan-tree-dump-times "MIN_EXPR" 2 "phiopt1" } } */ +/* { dg-final { cleanup-tree-dump "phiopt1" } } */ + Index: passes.c =================================================================== --- passes.c (revision 183396) +++ passes.c (working copy) @@ -1302,9 +1302,11 @@ init_optimization_passes (void) NEXT_PASS (pass_copy_prop); NEXT_PASS (pass_merge_phi); NEXT_PASS (pass_vrp); + NEXT_PASS (pass_phi_only_cprop); NEXT_PASS (pass_dce); NEXT_PASS (pass_cselim); NEXT_PASS (pass_tree_ifcombine); + NEXT_PASS (pass_merge_phi); NEXT_PASS (pass_phiopt); NEXT_PASS (pass_tail_recursion); NEXT_PASS (pass_ch); @@ -1401,6 +1403,7 @@ init_optimization_passes (void) NEXT_PASS (pass_late_warn_uninitialized); NEXT_PASS (pass_dse); NEXT_PASS (pass_forwprop); + NEXT_PASS (pass_merge_phi); NEXT_PASS (pass_phiopt); NEXT_PASS (pass_fold_builtins); NEXT_PASS (pass_optimize_widening_mul);