[PATCH] Fix old problem in if-conversion pass

[Eric Botcazou]

Hi,

We've run into a problematic piece of code in ifcvt.c that has been there from 
the day the file was added.  It's in noce_try_abs:

  /* Verify that C is zero.  Search backward through the block for
     a REG_EQUAL note if necessary.  */
  if (REG_P (c))
    {
      rtx insn, note = NULL;
      for (insn = earliest;
	   insn != BB_HEAD (if_info->test_bb);
	   insn = PREV_INSN (insn))
	if (INSN_P (insn)
	    && ((note = find_reg_note (insn, REG_EQUAL, c))
		|| (note = find_reg_note (insn, REG_EQUIV, c))))
	  break;
      if (! note)
	return FALSE;
      c = XEXP (note, 0);
    }

This code triggers an ICE if 'earliest' doesn't live in the same BB as the 
jump (because of -fnon-call-exceptions) as the condition

 insn != BB_HEAD (if_info->test_bb);

is never true.  The minimal fix could be to avoid searching backwards if 
'earliest' doesn't live in the same BB as the jump.  However it turns out that 
the whole loop is broken:
- it only looks for notes, potentially missing simple sets,
- it invokes find_reg_note (insn, REG_EQUAL, c) but 'c' is a register so it
looks for notes that reference a pseudo! Since the code just below
essentially accepts only the constant 0, the loop is dead and FALSE is always
returned, i.e. it cannot even optimize

float foo(float f)
{
  if (f < 0.0)
    f = -f;

  return f;
}

at -O -march=i686:

foo:
        pushl   %ebp
        movl    %esp, %ebp
        flds    8(%ebp)
        fld     %st(0)
        fchs
        fldz
        fucomip %st(2), %st
        fcmovbe %st(1), %st
        fstp    %st(1)
        popl    %ebp
        ret

I think the correct approach is to look only at the insn immediately preceding 
'earliest'.  This idiom is used elsewhere (noce_get_alt_condition).

But there is yet another problem: a 'neg' is emitted right after the 'abs', 
that is the above C code is turned into:

float foo(float f)
{
  return - fabs (f);
}

The problem is that noce_try_abs forgets to negate the transformation when the
comparison (f < 0.0) is written as (0.0 > f).


With the attached patch, the code generated at -O -march=i686 for the above C 
testcase is:

foo:
        pushl   %ebp
        movl    %esp, %ebp
        flds    8(%ebp)
        fabs
        popl    %ebp
        ret

Bootstrapped/regtested on x86_64-suse-linux.  OK for mainline?


2005-11-08  Eric Botcazou  <ebotcazou@adacore.com>

	* ifcvt.c (noce_try_abs): Negate if the comparison is reversed.
	Look only one instruction backwards for a REG_EQUAL note.


-- 
Eric Botcazou
-------------- next part --------------
A non-text attachment was scrubbed...
Name: e412-010_fsf.diff
Type: text/x-diff
Size: 1874 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20051108/081d3b32/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fabs.c
Type: text/x-csrc
Size: 98 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20051108/081d3b32/attachment-0001.bin>