New CSE2 bug on release branch demonstrated by g77

New CSE2 bug on release branch demonstrated by g77

[David Edelsohn]

	The G77 testsuite has a recent failure which I have tracked down
to a CSE2 bug.  Follow (reg:DF 33 f1) in the RTL below.

	Compiling erfc.f with -O2, the last call to erfc() originally
looks like:

(call_insn/u 244 243 246 (parallel[ 
            (set (reg:DF 33 f1)
                (call (mem:SI (symbol_ref:SI ("erfc[DS]")) 0)
                    (const_int 32 [0x20])))
            (use (const_int 0 [0x0]))
            (clobber (scratch:SI))
        ] ) -1 (nil)
    (expr_list:REG_EH_REGION (const_int -1 [0xffffffff])
        (nil))
    (expr_list (use (reg:DF 33 f1))
        (expr_list (use (reg:DF 3 r3))
            (nil))))

(insn 246 244 248 (set (reg:DF 139)
        (reg:DF 33 f1)) -1 (nil)
    (insn_list:REG_RETVAL 241 (expr_list:REG_EQUAL (expr_list (symbol_ref:SI ("e
rfc[DS]"))
                (expr_list (reg/v:DF 84)
                    (nil)))
            (nil))))

(insn 248 246 255 (set (reg/v:DF 85)
        (reg:DF 139)) -1 (nil)
    (nil))

***** After CSE1:

(call_insn/u 244 243 246 (parallel[ 
            (set (reg:DF 33 f1)
                (call (mem:SI (symbol_ref:SI ("erfc[DS]")) 0)
                    (const_int 32 [0x20])))
            (use (const_int 0 [0x0]))
            (clobber (scratch:SI))
        ] ) 347 {*call_value_nonlocal_aix32} (nil)
    (expr_list:REG_EH_REGION (const_int -1 [0xffffffff])
        (nil))
    (expr_list (use (reg:DF 33 f1))
        (expr_list (use (reg:DF 3 r3))
            (nil))))

(insn 246 244 257 (set (reg/v:DF 85)
        (reg:DF 33 f1)) 289 {*movdf_hardfloat32} (nil)
    (insn_list:REG_RETVAL 241 (expr_list:REG_EQUAL (expr_list (symbol_ref:SI ("e
rfc[DS]"))
                (expr_list (reg/v:DF 84)
                    (nil)))
            (nil))))


***** After CSE2:

(call_insn/u 244 243 246 (parallel[ 
            (set (reg:DF 33 f1)
                (call (mem:SI (symbol_ref:SI ("erfc[DS]")) 0)
                    (const_int 32 [0x20])))
            (use (const_int 0 [0x0]))
            (clobber (scratch:SI))
        ] ) 347 {*call_value_nonlocal_aix32} (nil)
    (expr_list:REG_EH_REGION (const_int -1 [0xffffffff])
        (nil))
    (expr_list (use (reg:DF 33 f1))
        (expr_list (use (reg:DF 3 r3))
            (nil))))

(insn 246 244 257 (set (reg/v:DF 85)
        (reg/v:DF 85)) 289 {*movdf_hardfloat32} (nil)
    (insn_list:REG_RETVAL 241 (expr_list:REG_EQUAL (expr_list (symbol_ref:SI ("e
rfc[DS]"))
                (expr_list (reg/v:DF 84)
                    (nil)))
            (nil))))

Pseudo 85 now is a move to itself and hard reg 33 is unused.  This is
confirmed by the Life pass:

(call_insn/u 244 243 246 (parallel[ 
            (set (reg:DF 33 f1)
                (call (mem:SI (symbol_ref:SI ("erfc[DS]")) 0)
                    (const_int 32 [0x20])))
            (use (const_int 0 [0x0]))
            (clobber (scratch:SI))
        ] ) 347 {*call_value_nonlocal_aix32} (insn_list 241 (insn_list 243 (nil)
))
    (expr_list:REG_DEAD (reg:DF 3 r3)
        (expr_list:REG_UNUSED (reg:DF 33 f1)
            (expr_list:REG_UNUSED (scratch:SI)
                (expr_list:REG_EH_REGION (const_int -1 [0xffffffff])
                    (nil)))))
    (expr_list (use (reg:DF 33 f1))
        (expr_list (use (reg:DF 3 r3))
            (nil))))

We then end up using pseudo 85 from the previous block and compare
garbage. 

David

Re: New CSE2 bug on release branch demonstrated by g77

[Mark Mitchell]

Thank you for the report.

Are you able to isolate the change that caused it?

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: New CSE2 bug on release branch demonstrated by g77

[David Edelsohn]

>>>>> Mark Mitchell writes:

Mark> Thank you for the report.
Mark> Are you able to isolate the change that caused it?

	I only just found the failure and tracked it this far.  Nothing
seems to have changed in cse.c recently.

	This only happens with -O2, not -O, although CSE2 appears to be
run at any optimization.

	I assume that some information being used by CSE2 is wrong.

David

Re: New CSE2 bug on release branch demonstrated by g77

[David Edelsohn]

>>>>> Mark Mitchell writes:

Mark> Thank you for the report.
Mark> Are you able to isolate the change that caused it?

	The bug appears to be due to your Stepanov fix:

        * toplev.c (rest_of_compilation): Remove dead code before
        purge_addressof.

I wonder if Andreas's bootstrap failure is caused by this as well.

	Unless we can track down the reason this patch is causing a
problem, it looks like GCC 3.0 will have bad C++ performance for this type
of code.

David

Re: New CSE2 bug on release branch demonstrated by g77

[Mark Mitchell]

--On Tuesday, June 12, 2001 06:04:43 PM -0400 David Edelsohn 
<dje@watson.ibm.com> wrote:

>>>>>> Mark Mitchell writes:
>
> Mark> Thank you for the report.
> Mark> Are you able to isolate the change that caused it?
>
> 	The bug appears to be due to your Stepanov fix:
>
>         * toplev.c (rest_of_compilation): Remove dead code before
>         purge_addressof.
>
> I wonder if Andreas's bootstrap failure is caused by this as well.

Thank you for tracking it down.  I will try to understand what's
going on -- it most likely reflects a bug somewhere else.  Unless
it looks very, very easy to fix, I will plan on reverting my patch,
on the branch, and doing whatever turns about to be appropriate
(revert, or install a more complex fix) on the mainline.

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: New CSE2 bug on release branch demonstrated by g77

[Mark Mitchell]

I understand what is going wrong now.

The important part is:

  (insn 232 299 237 (set (reg/v:DF 84)
        (const_double:DF (const_int 0 [0x0]) 0 [0x0] 1076101120 
[0x40240000] [10])) -1 (nil)
    (nil))

  ...

  (insn 246 244 248 (set (reg:DF 139)
        (reg:DF 33 f1)) -1 (nil)
    (insn_list:REG_RETVAL 241 (expr_list:REG_EQUAL (expr_list 
(symbol_ref:SI     ("erfc[DS]"))
                (expr_list (reg/v:DF 84)
                    (nil)))
            (nil))))

The additional call to life_analysis removes insn 232, which is
dead, but does not update the REG_EQUAL note.  (This bizarre-looking
REG_EQUAL note is inserted by the libcall-emission code; it represents
the entire libcall sequence.)  So, CSE2 gets confused.

There does not seem to be an easy fix.  There's no easy way to
even know which REG_EQUAL notes are talking about which instructions;
if we scanned the instruction stream, we would have a hard time
knowing which notes to remove.  But leaving the dead instruction
in is pretty silly, too.

Richard do you have any good ideas?

In the meanwhile, I am going to revert my patch, and reopen the PR,
downgraded to medium priority.

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: New CSE2 bug on release branch demonstrated by g77

[Richard Henderson]

On Tue, Jun 12, 2001 at 07:58:46PM -0700, Mark Mitchell wrote:
> Richard do you have any good ideas?

I'm wondering why CSE1 didn't substitute that const_double
into the REG_EQUAL.  Something worth looking into, but I'm
not planning on doing it right now.


r~

Re: New CSE2 bug on release branch demonstrated by g77

[Mark Mitchell]

--On Tuesday, June 12, 2001 08:52:51 PM -0700 Richard Henderson 
<rth@redhat.com> wrote:

> On Tue, Jun 12, 2001 at 07:58:46PM -0700, Mark Mitchell wrote:
>> Richard do you have any good ideas?
>
> I'm wondering why CSE1 didn't substitute that const_double
> into the REG_EQUAL.  Something worth looking into, but I'm
> not planning on doing it right now.

Me neither.  Yes, that's an interesting question.

Thanks,

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com