This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Fix twolf -funroll-loops -O3 miscompilation (a semi-latent web.c bug)


On Wed, Nov 28, 2012 at 8:44 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> Again: Is this really the direction we should be going in? (I haven't
>> any better ideas...)
>
> Do you mean for self-referencing notes?  If so, definitely, these notes are
> either wrong or almost always redundant so we should eliminate them.

Well, I'm not sure I agree that they are wrong. Consider:

S0: r1 = ...
S1: r2 = r1 + 10
S2: r1 = r2 + 10 { REG_EQUAL r1 + 20 }
S3: r3 = r1 + 10

Clearly it would be wrong to use the REG_EQUAL note on S2 to optimize S3 to:

S0: r1 = ...
S1: r2 = r1 + 10
S2: r1 = r1 + 20
S3: r3 = r1 + 30

However, It seems to me that this would be valid if e.g. the webizer
turns the above into:

S0: r1 = ...
S1: r2 = r1 + 10
S2: r4 = r2 + 10 { REG_EQUAL r1 + 20 }
S3: r3 = r4 + 10

because now the optimization would be valid:

S0: r1 = ...
S1: r2 = r1 + 10
S2: r4 = r1 + 20
S3: r3 = r1 + 30


So self-referencing REG_EQUAL notes should IMHO be considered valid,
and transformations using the notes should just be careful to
invalidate the equivalence before processing the insn that the note
appears on. cse.c doesn't appear to do this, however.


On a completely different note:

df-problems.c has this comment:

            /* Remove the notes that refer to dead registers.  As we
have at most
               one REG_EQUAL/EQUIV note, all of EQ_USES will refer to this note
               so we need to purge the complete EQ_USES vector when removing
               the note using df_notes_rescan.  */

But ira.c:update_equiv_regs creates insns with a REG_EQUAL *and* a
REG_EQUIV note:
(gdb)
update_equiv_regs () at ../../trunk/gcc/ira.c:3177
3177                    = gen_rtx_INSN_LIST (VOIDmode, insn, NULL_RTX);
2: debug_rtx((rtx)0x7ffff4df5e58) = (insn 638 637 639 85 (parallel [
            (set (reg:SI 891)
                (minus:SI (reg:SI 890)
                    (reg:SI 546 [ D.45472 ])))
            (clobber (reg:CC 17 flags))
        ]) ../../trunk/gcc/value-prof.c:928 283 {*subsi_1}
     (expr_list:REG_EQUIV (mem:SI (plus:DI (reg/v/f:DI 204 [ e12 ])
                (const_int 52 [0x34])) [4 e12_223->probability+0 S4 A32])
        (expr_list:REG_UNUSED (reg:CC 17 flags)
            (expr_list:REG_EQUAL (minus:SI (const_int 10000 [0x2710])
                    (reg:SI 546 [ D.45472 ]))
                (nil)))))
void
(gdb)

Is that valid?

Ciao!
Steven




> As for the more general problem of REG_EQ* notes, perhaps it's time to devise
> a global infrastructure to handle them.  But, as far as I recall, they always
> have been problematic, before or after the DF merge.

            /* Remove the notes that refer to dead registers.  As we
have at most
               one REG_EQUAL/EQUIV note, all of EQ_USES will refer to this note
               so we need to purge the complete EQ_USES vector when removing
               the note using df_notes_rescan.  */


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]