On Wed, Feb 4, 2015 at 12:28 AM, Jeff Law <l...@redhat.com> wrote: > On 02/03/15 01:29, Bin.Cheng wrote: >> >> >> Hmm, if I understand correctly, it's a code size regression, so I >> don't think it's appropriate to adapt the test case. Either the patch >> or something else in GCC is doing wrong, right? >> >> Hi Alex, could you please file a PR with full dump information for >> tracking? > > But if the code size regression is due to the older compiler incorrectly > handling the promotion of REG_EQUAL to REG_EQUIV notes, then the test > absolutely does need updating as the codesize was dependent on incorrect > behaviour in the compiler.
Hi Jeff, I looked into the test and can confirm the previous compilation is correct. The cover letter of this patch said IRA mis-handled REQ_EQUIV before, but in this case it is REG_EQUAL that is lost. The full dump (without this patch) after IRA is like: 10: NOTE_INSN_BASIC_BLOCK 2 2: r116:SI=r0:SI 3: r117:SI=r1:SI REG_DEAD r1:SI 4: r118:SI=r2:SI REG_DEAD r2:SI 5: NOTE_INSN_FUNCTION_BEG 12: r2:SI=0x1 13: r1:SI=0 15: r0:SI=call [`lseek'] argc:0 REG_DEAD r2:SI REG_DEAD r1:SI REG_CALL_DECL `lseek' 16: r111:SI=r0:SI REG_DEAD r0:SI 17: r2:SI=0x2 18: r1:SI=0 19: r0:SI=r116:SI REG_DEAD r116:SI 20: r0:SI=call [`lseek'] argc:0 REG_DEAD r2:SI REG_DEAD r1:SI REG_CALL_DECL `lseek' 21: r112:SI=r0:SI REG_DEAD r0:SI 22: cc:CC=cmp(r111:SI,0xffffffffffffffff) 23: pc={(cc:CC==0)?L46:pc} REG_DEAD cc:CC REG_BR_PROB 159 24: NOTE_INSN_BASIC_BLOCK 3 25: cc:CC=cmp(r112:SI,0xffffffffffffffff) 26: pc={(cc:CC==0)?L50:pc} REG_DEAD cc:CC REG_BR_PROB 159 27: NOTE_INSN_BASIC_BLOCK 4 28: NOTE_INSN_DELETED 29: {cc:CC_NOOV=cmp(r112:SI-r111:SI,0);r114:SI=r112:SI-r111:SI;} REG_DEAD r112:SI 30: pc={(cc:CC_NOOV==0)?L54:pc} REG_DEAD cc:CC_NOOV REG_BR_PROB 400 31: NOTE_INSN_BASIC_BLOCK 5 32: [r117:SI]=r111:SI REG_DEAD r117:SI REG_DEAD r111:SI 33: [r118:SI]=r114:SI REG_DEAD r118:SI REG_DEAD r114:SI 7: r110:SI=0 REG_EQUAL 0 76: pc=L34 77: barrier 46: L46: 45: NOTE_INSN_BASIC_BLOCK 6 8: r110:SI=r111:SI REG_DEAD r111:SI REG_EQUAL 0xffffffffffffffff 78: pc=L34 79: barrier 50: L50: 49: NOTE_INSN_BASIC_BLOCK 7 6: r110:SI=r112:SI REG_DEAD r112:SI REG_EQUAL 0xffffffffffffffff 80: pc=L34 81: barrier 54: L54: 53: NOTE_INSN_BASIC_BLOCK 8 9: r110:SI=0xffffffffffffffff REG_EQUAL 0xffffffffffffffff 34: L34: 35: NOTE_INSN_BASIC_BLOCK 9 40: r0:SI=r110:SI REG_DEAD r110:SI 41: use r0:SI Before r216169 (with REG_EQUAL in insn9), jumps from basic block 6/7/8 -> 9 can be merged because r110 equals to -1 afterwards. But with the patch, the equal information of r110==-1 in basic block 8 is lost. As a result, jump from 8->9 can't be merged and two additional instructions are generated. I suppose the REG_EQUAL note is correct in insn9? According to GCCint, it only means r110 set by insn9 will be equal to the value at run time at the end of this insn but not necessarily elsewhere in the function. I also found another problem (or mis-leading?) with the document: "Thus, compiler passes prior to register allocation need only check for REG_EQUAL notes and passes subsequent to register allocation need only check for REG_EQUIV notes". This seems not true now as in this example, passes after register allocation do take advantage of REG_EQUAL in optimization and we can't achieve that by using REG_EQUIV. Thanks, bin > > jeff