[PATCH] Fix PR30773 and its duplicates

[Richard Guenther]

On 2/18/07, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> Hello,
>
> My REG_EQ* patch of last week caused bootstrap failures on a number of
> x86 targets, and it also caused SPEC failures and one test suite failure
> on i686.  The patch below fixes all those issues.
>
> The problem is that we used to set a REG_EQUIV note on an insn with more
> than one set.  Diff of the lreg dump before and after the patch shows
> this:
>
>  (insn:HI 136 135 137 16 (parallel [
>              (set (reg:SI 100)
>                  (div:SI (reg:SI 98)
>                      (mem/s:SI (plus:SI (reg/v/f:SI 85 [ kb ])
>                              (const_int 8 [0x8])) [9 <variable>.nfr+0 S4A32])))
>              (set (reg:SI 101)
>                  (mod:SI (reg:SI 98)
>                      (mem/s:SI (plus:SI (reg/v/f:SI 85 [ kb ])
>                              (const_int 8 [0x8])) [9 <variable>.nfr+0 S4A32])))
>              (clobber (reg:CC 17 flags))
>          ]) 279 {*divmodsi4_nocltd} (insn_list:REG_DEP_TRUE 135 (nil))
> -    (expr_list:REG_EQUIV (mem:SI (plus:SI (reg/f:SI 7 sp)
> -                (const_int 12 [0xc])) [0 S4 A32])
> -        (expr_list:REG_DEAD (reg:SI 98)
> -            (expr_list:REG_UNUSED (reg:CC 17 flags)
> -                (expr_list:REG_UNUSED (reg:SI 101)
> -                    (nil))))))
> +    (expr_list:REG_DEAD (reg:SI 98)
> +        (expr_list:REG_UNUSED (reg:CC 17 flags)
> +            (expr_list:REG_UNUSED (reg:SI 101)
> +                (nil)))))
>
> We used to set the REG_EQUIV note by just chaining it to the REG_NOTES
> list, in local-alloc.c:update_equiv_regs().  This function assumes it
> is OK to attach this REG_EQUIV note because the insn it attaches it to
> only has a single set according to single_set().  But this is wrong,
> single set returns the only *useful* set of an insn.  Insn 136 that I
> showed above only has one useful set because of the REG_UNUSED (reg 101)
> note, so single_set() returns the set of reg 100 as the single useful
> set of this insn.
>
> But quoting emit-rtl.c:set_unique_reg_note():
>
>     case REG_EQUAL:
>     case REG_EQUIV:
>       /* Don't add REG_EQUAL/REG_EQUIV notes if the insn
>          has multiple sets (some callers assume single_set
>          means the insn only has one set, when in fact it
>          means the insn only has one * useful * set).  */
>       if (GET_CODE (PATTERN (insn)) == PARALLEL && multiple_sets (insn))
>         {
>           gcc_assert (!note);
>           return NULL_RTX;
>         }
>
> So, setting the REG_EQUIV note on the *divmodsi4_nocltd insn is wrong,
> and it was a bug in local-alloc.c.  Now that we use set_unique_reg_note
> there, we do not add the REG_EQUIV note in update_equiv_regs.  From
> there, things begin to break: We *do* set reg_equiv_init, but because
> there is no REG_EQUIV note, we make it poitn to an inappropriate insn,
> which we later delete during reload.  This causes all the reported
> failures.
>
> The patch below fixes these problems by not setting reg_equiv_init if
> we fail to attach the REG_EQUIV note.
>
> The patch may cause some missed optimizations, that I can unfortunately
> not do much about.  The only I can see to way to fix these, is to add
> splitters to the i386 backend to split the divmod* insns into just div
> or mod when one of the results of the insn is unused.  The splitted
> insn will have only one set, so that attaching a REG_EQUIV note would be
> valid.  I'll leave this to an i386 maintainer, because these divmod
> insn definitions are already quite complex, they boggle my mind ;-)
>
> This patch was bootstrapped+tested on i686-pc-linux-gnu.  The patch
> fixes the simd-1.c failures and does not cause new failures.
> OK for the trunk?

Ok.  Mind opening a PR for the missed optimization?

Thanks,
Richard.

> Gr.
> Steven
>
>
>         * local-alloc.c (update_equiv_regs): Do not set reg_equiv_init
>         if we fail to attach a REG_EQUIV note.