From r127766 it is evident that mentioned commit fixed obvious problem that has re-enabled a lot of ifcvt functionality. The bug is in this re-enabled part of ifcvt. But we need a testcase here.

[hjl@gnu-28 rrs]$ cat cmov.cc
#include <stdlib.h>

const double infinity = 1e100;
enum Status
{
  P_ON_LOWER = -4,
  P_ON_UPPER = -2,
  P_FREE = -1,
  P_FIXED = P_ON_UPPER + P_ON_LOWER
};

void
foo (enum Status & stat, double newUpper, double lower)
{
  switch (stat)
  {
  case P_ON_UPPER:
      if (newUpper >= infinity)
         stat = (lower <= -infinity)
            ? P_FREE
            : P_ON_LOWER;
      else if (newUpper == lower)
         stat = P_FIXED;
      break;
  default:
      abort();
  }
}

int
main ()
{
  enum Status stat = P_ON_UPPER;
  foo (stat, 5.000000e+01, -1.000000e+100);
  if (stat != P_ON_UPPER)
    abort ();
  return 0;
}
[hjl@gnu-28 rrs]$ /export/gnu/import/rrs/127766/usr/bin/g++ -static -O2 cmov.cc
[hjl@gnu-28 rrs]$ ./a.out
Aborted
[hjl@gnu-28 rrs]$ /export/gnu/import/rrs/127765/usr/bin/g++ -static -O2 cmov.cc
[hjl@gnu-28 rrs]$ ./a.out
[hjl@gnu-28 rrs]$ /export/gnu/import/rrs/127766/usr/bin/g++ -static -O2 cmov.cc -march=i686 -m32
[hjl@gnu-28 rrs]$ ./a.out
Aborted
[hjl@gnu-28 rrs]$ /export/gnu/import/rrs/127766/usr/bin/g++ -static -O2 cmov.cc -m32
[hjl@gnu-28 rrs]$ ./a.out
[hjl@gnu-28 rrs]$

A smaller testcase:

[hjl@gnu-28 cmov-1]$ cat cmov.cc
extern "C" void abort (void);

enum Status
{
  P_ON_LOWER = -4,
  P_ON_UPPER = -2,
  P_FREE = -1,
};

void
foo (enum Status & stat, double newUpper, double lower, double max)
{
  if (stat == P_ON_UPPER)
    {
      if (newUpper >= max)
        stat = (lower <= -max) ? P_FREE : P_ON_LOWER;
      else if (newUpper == lower)
        stat = P_ON_LOWER;
    }
}

int
main ()
{
  enum Status stat = P_ON_UPPER;
  foo (stat, 5.000000e+01, -1.000000e+100, 1e100);
  if (stat != P_ON_UPPER)
    abort ();
  return 0;
}
[hjl@gnu-28 cmov-1]$ make
/export/gnu/import/rrs/127766/usr/bin/g++ -O2 -g   -c -o cmov.o cmov.cc
/export/gnu/import/rrs/127766/usr/bin/g++ -static -o cmov cmov.o
./cmov
make: *** [all] Aborted
[hjl@gnu-28 cmov-1]$

C testcase:

--cut here--
extern void abort (void);

enum Status
{
  P_ON_LOWER = -4,
  P_ON_UPPER = -2,
  P_FREE = -1
};

void
foo (enum Status *stat, double newUpper, double lower, double max)
{
  if (newUpper >= max)
    *stat = (lower <= -max) ? P_FREE : P_ON_LOWER;
  else if (newUpper == lower)
    *stat = P_ON_LOWER;
}

int
main ()
{
  enum Status stat = P_ON_UPPER;

  foo (&stat, 5.0, -10.0, 10.0);

  if (stat != P_ON_UPPER)
    abort ();
  return 0;
}
--cut here--

For some reason, ifcvt converts 

(note 32 28 33 7 [bb 7] NOTE_INSN_BASIC_BLOCK)

(insn 33 32 34 7 pr33181.c:15 (set (reg:CCFP 17 flags)
        (compare:CCFP (reg/v:DF 60 [ newUpper ])
            (reg/v:DF 61 [ lower ]))) 35 {*cmpfp_i_sse} (expr_list:REG_DEAD (reg
/v:DF 61 [ lower ])
        (expr_list:REG_DEAD (reg/v:DF 60 [ newUpper ])
            (nil))))

(jump_insn 34 33 54 7 pr33181.c:15 (set (pc)
        (if_then_else (uneq (reg:CCFP 17 flags)
                (const_int 0 [0x0]))
            (label_ref:DI 54)
            (pc))) 563 {*jcc_1} (expr_list:REG_DEAD (reg:CCFP 17 flags)
        (expr_list:REG_BR_PROB (const_int 5400 [0x1518])
            (nil))))
;; End of basic block 7 -> ( 8 9)

;; Start of basic block ( 7) -> 8

(code_label 54 34 38 8 10 "" [1 uses])

(insn 39 38 45 8 pr33181.c:16 (set (mem:SI (reg/v/f:DI 59 [ stat ]) [2 S4 A32])
        (const_int -4 [0xfffffffffffffffc])) 47 {*movsi_1} (expr_list:REG_DEAD (
reg/v/f:DI 59 [ stat ])
        (nil)))

;; End of basic block 8 -> ( 9)

**** into ****

(insn 33 32 62 7 pr33181.c:15 (set (reg:CCFP 17 flags)
        (compare:CCFP (reg/v:DF 60 [ newUpper ])
            (reg/v:DF 61 [ lower ]))) 35 {*cmpfp_i_sse} (expr_list:REG_DEAD (reg
/v:DF 61 [ lower ])
        (expr_list:REG_DEAD (reg/v:DF 60 [ newUpper ])
            (nil))))

(insn 62 33 61 7 pr33181.c:16 (set (reg:SI 66)
        (const_int -4 [0xfffffffffffffffc])) 47 {*movsi_1} (nil))

(insn 61 62 63 7 pr33181.c:16 (set (reg:CCFP 17 flags)
        (compare:CCFP (reg/v:DF 60 [ newUpper ])
            (reg/v:DF 61 [ lower ]))) 35 {*cmpfp_i_sse} (nil))

(insn 63 61 64 7 pr33181.c:16 (set (reg:SI 65)
        (if_then_else:SI (ltgt (reg:CCFP 17 flags)
                (const_int 0 [0x0]))
            (reg:SI 58 [ iftmp.0 ])
            (reg:SI 66))) 798 {*movsicc_noc} (nil))

(insn 64 63 45 7 pr33181.c:16 (set (mem:SI (reg/v/f:DI 59 [ stat ]) [2 S4 A32])
        (reg:SI 65)) 47 {*movsi_1} (nil))

**** this conversionis wrong, beacuse iftmp.0 is uninitialized in this BB. To compensate this, df initializes iftmp.0 to zero, resulting in:

.L11:
        movl    $-4, %eax       #, tmp66
        movl    $0, %edx        #, iftmp.0
        comisd  %xmm1, %xmm0    # lower, newUpper
        cmovne  %edx, %eax      # iftmp.0,, tmp66
        movl    %eax, (%rdi)    # tmp66,* stat
        ret

instead of:

.L2:
        comisd  %xmm1, %xmm0    # lower, newUpper
        jne     .L12    #,
        movl    $-4, (%rdi)     #,* stat
.L12:
        rep ; ret

The problem is in the way ifcvt handles IFs without ELSE blocks. Compiling c testcase with -O2 -ffast-math (on x86_64 target), we hit check for else_bb in noce_process_if_block(). We continue into line 2196 of ifcvt.c, where we try to find a store before condition using:

      prev_insn = prev_nonnote_insn (if_info->cond_earliest);

The problem is, that in gcc-4.3, we have:

;; Pred edge  5 [100.0%]  (fallthru)
;; Pred edge  4 [100.0%]  (fallthru)
(code_label 26 25 27 6 5 "" [0 uses])

(note 27 26 28 6 [bb 6] NOTE_INSN_BASIC_BLOCK)

(insn 28 27 32 6 pr33181.c:14 (set (mem:SI (reg/v/f:DI 59 [ stat ]) [2 S4 A32])
        (reg:SI 58 [ iftmp.0 ])) 47 {*movsi_1} (expr_list:REG_DEAD (reg/v/f:DI 59 [ stat ])
        (expr_list:REG_DEAD (reg:SI 58 [ iftmp.0 ])
            (nil))))
;; End of basic block 6 -> ( 9)
;; lr  out       6 [bp] 7 [sp] 16 [argp] 20 [frame]
;; live  out     6 [bp] 7 [sp] 16 [argp] 20 [frame]


;; Succ edge  9 [100.0%]  (fallthru)

;; Start of basic block ( 2) -> 7
;; bb 7 artificial_defs: { }
;; bb 7 artificial_uses: { u35(6){ }u36(7){ }u37(16){ }u38(20){ }}
;; lr  in        6 [bp] 7 [sp] 16 [argp] 20 [frame] 59 60 61
;; lr  use       6 [bp] 7 [sp] 16 [argp] 20 [frame] 60 61
;; lr  def       17 [flags]
;; live  in      6 [bp] 7 [sp] 16 [argp] 20 [frame] 59 60 61
;; live  gen     17 [flags]
;; live  kill

;; Pred edge  2 [50.0%]  (fallthru)
(note 32 28 33 7 [bb 7] NOTE_INSN_BASIC_BLOCK)

(insn 33 32 34 7 pr33181.c:15 (set (reg:CCFP 17 flags)
        (compare:CCFP (reg/v:DF 60 [ newUpper ])
            (reg/v:DF 61 [ lower ]))) 35 {*cmpfp_i_sse} (expr_list:REG_DEAD (reg/v:DF 61 [ lower ])
        (expr_list:REG_DEAD (reg/v:DF 60 [ newUpper ])
            (nil))))

So, previous non-note insn from (insn 33) is (insn 28)(!!) in a totally unrelated BB, leading to all sort of strange things.

For comparison, in gcc-4.1 we have a lot cleaner sequence:

--cut here--
;; Start of basic block 5, registers live: (nil)
(code_label 43 42 44 5 2 "" [1 uses])

(note 44 43 46 5 [bb 5] NOTE_INSN_BASIC_BLOCK)

(insn 46 44 47 5 (set (reg:CCFP 17 flags)
        (compare:CCFP (reg/v:DF 60 [ newUpper ])
            (reg/v:DF 61 [ lower ]))) 28 {*cmpfp_i_sse} (nil)
    (nil))

(jump_insn 47 46 52 5 (set (pc)
        (if_then_else (ltgt (reg:CCFP 17 flags)
                (const_int 0 [0x0]))
            (label_ref:DI 62)
            (pc))) 531 {*jcc_1} (nil)
    (expr_list:REG_BR_PROB (const_int 3300 [0xce4])
        (nil)))
;; End of basic block 5, registers live:
 (nil)

;; Start of basic block 6, registers live: (nil)
(note 52 47 54 6 [bb 6] NOTE_INSN_BASIC_BLOCK)

(insn 54 52 57 6 (set (mem:SI (reg/v/f:DI 59 [ stat ]) [2 S4 A32])
        (const_int -4 [0xfffffffffffffffc])) 40 {*movsi_1} (nil)
    (nil))
;; End of basic block 6, registers live:
 (nil)

(note 57 54 62 NOTE_INSN_FUNCTION_END)

;; Start of basic block 7, registers live: (nil)
(code_label 62 57 71 7 12 "" [2 uses])

(note 71 62 0 7 [bb 7] NOTE_INSN_BASIC_BLOCK)
;; End of basic block 7, registers live:
 (nil)
--cut here--

And previous non-note insn of the condition is (code label 43) that correctly blocks cmove conversion.

Perhaps Jan has an advice on prev_nonnote_insn() usage in noce_process_if_block().

This patch

2007-08-26  H.J. Lu  <hongjiu.lu@intel.com>

        PR middle-end/33181
        * ifcvt.c (noce_process_if_block): Don't move insn across
        basic block.

--- gcc/ifcvt.c.cmov    2007-08-24 06:02:57.000000000 -0700
+++ gcc/ifcvt.c 2007-08-26 07:35:50.000000000 -0700
@@ -2198,6 +2198,7 @@ noce_process_if_block (struct noce_if_in
         COND_EARLIEST to JUMP.  Make sure the relevant data is still
         intact.  */
       if (! insn_b
+         || BLOCK_NUM (insn_b) != BLOCK_NUM (if_info->cond_earliest)
          || !NONJUMP_INSN_P (insn_b)
          || (set_b = single_set (insn_b)) == NULL_RTX
          || ! rtx_equal_p (x, SET_DEST (set_b))

works for me. Does it make senses?

Subject: Re:  [4.3 Regression]  Revision 127766 generates
 bad cmov

On Sun, 26 Aug 2007, ubizjak at gmail dot com wrote:

> So, previous non-note insn from (insn 33) is (insn 28)(!!) in a totally
> unrelated BB, leading to all sort of strange things.

Since ifcvt is now working in cfg-layout mode basically all uses
of prev_nonnote_insn are busted.  I guess you could check if the
result is in the "right" BB, but ...

Subject: Re:  [4.3 Regression]  Revision 127766 generates
 bad cmov

On Sun, 26 Aug 2007, hjl at lucon dot org wrote:

> This patch
> 
> 2007-08-26  H.J. Lu  <hongjiu.lu@intel.com>
> 
>         PR middle-end/33181
>         * ifcvt.c (noce_process_if_block): Don't move insn across
>         basic block.
> 
> --- gcc/ifcvt.c.cmov    2007-08-24 06:02:57.000000000 -0700
> +++ gcc/ifcvt.c 2007-08-26 07:35:50.000000000 -0700
> @@ -2198,6 +2198,7 @@ noce_process_if_block (struct noce_if_in
>          COND_EARLIEST to JUMP.  Make sure the relevant data is still
>          intact.  */
>        if (! insn_b
> +         || BLOCK_NUM (insn_b) != BLOCK_NUM (if_info->cond_earliest)
>           || !NONJUMP_INSN_P (insn_b)
>           || (set_b = single_set (insn_b)) == NULL_RTX
>           || ! rtx_equal_p (x, SET_DEST (set_b))
> 
> works for me. Does it make senses?

Yes.  Or if we have domiantors available check if BB(insn_b) dominates
BB(if_info->cond_earliest).

Richard.

(In reply to comment #8)
>
> > 2007-08-26  H.J. Lu  <hongjiu.lu@intel.com>
> > 
> >         PR middle-end/33181
> >         * ifcvt.c (noce_process_if_block): Don't move insn across
> >         basic block.
> > 
...
> 
> Yes.  Or if we have domiantors available check if BB(insn_b) dominates
> BB(if_info->cond_earliest).
> 
> Richard.
> 

A patch is posted at

http://gcc.gnu.org/ml/gcc-patches/2007-08/msg01737.html

Subject: Bug 33181

Author: hjl
Date: Sun Aug 26 18:24:19 2007
New Revision: 127810

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=127810
Log:
gcc/

2007-08-26  H.J. Lu  <hongjiu.lu@intel.com>

	PR middle-end/33181
	* ifcvt.c (noce_get_alt_condition): Make sure that the previous
	non NOTE insn doesn't cross basic block.
	(noce_try_abs): Likewise.
	(noce_process_if_block): Likewise.

gcc/testsuite/

2007-08-26  H.J. Lu  <hongjiu.lu@intel.com>

	PR middle-end/33181
	* gcc.dg/ifelse-2.c: New.

Added:
    trunk/gcc/testsuite/gcc.dg/ifelse-2.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ifcvt.c
    trunk/gcc/testsuite/ChangeLog

Fixed.