https://gcc.gnu.org/bugzilla/show_bug.cgi?id=29333
rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |rsandifo at gcc dot gnu.org --- Comment #8 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> --- This may be worth filing as another PR (let me know if you think I should), but another case of VRP stymieing phiopt is: void bar (int); void foo (int a, int b) { if (!b) bar (1); else { int c; if (a) c = a; else c = 0; if (c == b) bar (2); } } Before vrp1 we have: <bb 2> [100.0%]: if (b_3(D) == 0) goto <bb 3>; [29.6%] else goto <bb 4>; [70.4%] <bb 3> [29.6%]: bar (1); goto <bb 8>; [100.0%] <bb 4> [70.4%]: if (a_4(D) != 0) goto <bb 6>; [50.0%] else goto <bb 5>; [50.0%] <bb 5> [35.2%]: <bb 6> [70.4%]: # c_1 = PHI <a_4(D)(4), 0(5)> if (c_1 == b_3(D)) goto <bb 7>; [22.9%] else goto <bb 8>; [77.0%] <bb 7> [16.2%]: bar (2); <bb 8> [100.0%]: return; phiopt would tell that c_1 is equal to a_4, and after making that substitution, cfgcleanup would remove blocks 4 and 5, leaving two comparisons rather than three. However, VRP runs first and threads 4->5 to 4->8, giving: <bb 2> [100.0%]: if (b_3(D) == 0) goto <bb 3>; [29.6%] else goto <bb 4>; [70.4%] <bb 3> [29.6%]: bar (1); goto <bb 7>; [100.0%] <bb 4> [70.4%]: if (a_4(D) != 0) goto <bb 5>; [50.0%] else goto <bb 8>; [50.0%] <bb 5> [35.2%]: # c_1 = PHI <a_4(D)(4)> if (c_1 == b_3(D)) goto <bb 6>; [45.9%] else goto <bb 7>; [54.1%] <bb 6> [16.2%]: bar (2); <bb 7> [100.0%]: return; <bb 8> [35.2%]: # c_11 = PHI <0(4)> goto <bb 7>; [100.0%] We never "recover" from that and so the three comparisons survive into the final output. Removing the first !b comparison leaves no jump threading opportunities, so phiopt kicks in as expected. This is a cut-down version of what happens with things like the safe_as_a calls in NEXT_INSN. E.g. in the basic-block iterator: #define FOR_BB_INSNS(BB, INSN) \ for ((INSN) = BB_HEAD (BB); \ (INSN) && (INSN) != NEXT_INSN (BB_END (BB)); \ (INSN) = NEXT_INSN (INSN)) we never get rid of the p null test in: template <typename T, typename U> inline T safe_as_a (U *p) { if (p) { gcc_checking_assert (is_a <T> (p)); return is_a_helper <T>::cast (p); } else return NULL; } even though in release builds the function should reduce to "return p".