Fix PR41087, ifcvt breakage for condition with postincrement

This is a regression since 4.5; latent since forever, exposed by
autocrement improvements.  In the case at hand, there was an
apparent trivial store_flag elimination opportunity (setting a
variable to 0 in the then-arm and to 1 in the else-arm), but
with a twist: the condition contained a (mem/s:SI (post_inc:SI
(reg:SI 241 [ ivtmp.23 ])) [3 S4 A8]) so when the full
store-flag sequence was emitted, *including a copy of the
compare insn*, the compare insn "stuck" (was not removed as dead
by later passes), supposedly because it had a side-effect.

My first inclination was that ifcvt should remove the compare
insn for the replaced conditional branch, but apparently ifcvt
is designed to emit the new sequence just before the old
conditional jump (after old compare insns), relying on later
passes to remove the (redundant) compare insn.  This *should*
work because the responsible function, noce_get_condition, is
supposed to be checking that "the resulting condition must be
valid at JUMP" (like, copies of it will be benign).  Which it
isn't when the condition has side-effects.  Besides, ifcvt
doesn't decorate the new compare insn with the necessary REG_INC
note that the next pass, regmove, expects.

One *could* fix this bug by forcing the offending part of the
condition to a register (pre-register-allocation), but that's a
bit too involved (e.g. it should be undone if the if-statement
after all can't be optimized but the insn-sequence would have to
be kept valid for other parts of ifcvt to work, so the register
can't be trivially substituted just in the insn but has to be
fully emitted as a move sequence).  Anyway, despite that the
failing test-case, gfortran.dg/zero_sized_3.f90 contained lots
of ifcvt-like conditional assignments after expansion and loop
unrolling, this miscompiled case was the only one trigging in
ifcvt at all for (not just post-combine).  Most simple ifcvt
cases seem already taken care of at expand time.  Point being,
an involved kind of solution doesn't seem worthwhile.  Better to
just make noce_get_condition do what it says.  As follows.  See
also PR11052, where a similar thing happened to the target (the
x in "if (c) x = a; else x = b;").

Regtested native and cross to cris-axis-elf (trunk; 4.5 in
progress).

Ok for trunk and 4.5?

gcc:
	PR rtl-optimization/41087
	* ifcvt.c (noce_get_condition): Don't allow conditions with
	side-effects.


brgds, H-P

On Fri, Sep 10, 2010 at 1:57 PM, Hans-Peter Nilsson
<hans-peter.nilsson@axis.com> wrote:
>        PR rtl-optimization/41087
>        * ifcvt.c (noce_get_condition): Don't allow conditions with
>        side-effects.
>
> Index: ifcvt.c
> ===================================================================
> --- ifcvt.c     (revision 164149)
> +++ ifcvt.c     (working copy)
> @@ -2201,8 +2201,15 @@ noce_get_condition (rtx jump, rtx *earli
>
>   /* Otherwise, fall back on canonicalize_condition to do the dirty
>      work of manipulating MODE_CC values and COMPARE rtx codes.  */
> -  return canonicalize_condition (jump, cond, reverse, earliest,
> -                                NULL_RTX, false, true);
> +  tmp = canonicalize_condition (jump, cond, reverse, earliest,
> +                               NULL_RTX, false, true);
> +
> +  /* We don't handle side-effects in the condition, like handling
> +     REG_INC notes and making sure no duplicate conditions are emitted.  */
> +  if (tmp != NULL_RTX && side_effects_p (tmp))
> +    return NULL_RTX;
> +
> +  return tmp;
>  }
>
>  /* Return true if OP is ok for if-then-else processing.  */
>

Can you not already see side effects _before_ canonicalize_condition?

Ciao!
Steven

> 	PR rtl-optimization/41087
> 	* ifcvt.c (noce_get_condition): Don't allow conditions with
> 	side-effects.

OK for trunk and 4.5 branch.