> 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
But the note is wrong by itself. The semantics is clear: the note means that r1 is equal to r1 + 20 right after S2, which doesn't make any sense. > 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 Sure, but we have no guarantee that the RTL stream will be massaged that way. > 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. IMO that's a recipe for bugs. > 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? Yes, the comment is wrong and should have been removed in r183719: http://gcc.gnu.org/ml/gcc-patches/2012-01/msg01547.html -- Eric Botcazou