When determining whether a move insn is a dead move or not, the code in removeDeadMove, doTermScan, scan4op (mcs51/peep.c) scans through the code following branches to find uses of the set register. It mostly works, but not in all cases it seems.
Examples have been compiled on 4.1.6 (svn r12557) with:
sdcc -c --std-sdcc2x -mmcs51 --model-large --stack-auto --fomit-frame-pointer
__data unsigned char xxx;
unsigned char bleh3 (void)
{
if ((xxx & 0x1F) == 0x10)
return 1;
else
return 3;
}
compiles to:
_bleh3:
mov a,#0x1f
anl a,_xxx
mov r7,a << unnecessary use of temporary 'r7'
cjne r7,#0x10,00102$ <<
mov dpl,#0x01
ret
00102$:
mov dpl,#0x03
ret
adding the following peephole pattern to try to remove the unnecessary temporary register use above:
replace {
mov r%1,a
cjne r%1,%2,%3
} by {
; Peephole 800.4
mov r%1,a
cjne a,%2,%3
}
compiles to:
_bleh3:
mov a,#0x1f
anl a,_xxx
mov r7,a << dead move after peephole not eliminated
cjne a,#0x10,00102$ << peephole is effective
mov dpl,#0x01
ret
00102$:
mov dpl,#0x03
ret
duplicating Peephole 302
pattern as a last peephole pattern, or setting the original Peephole 302
pattern as restart
will eliminate the dead move:
// duplicate simple dead move pattern as last peephole pattern
replace {
mov r%1,%2
} by {
; Peephole 302.1 mov r%1,%2 removed
} if notVolatile(%2), deadMove(%1)
compiles to:
_bleh3:
mov a,#0x1f
anl a,_xxx
cjne a,#0x10,00102$
mov dpl,#0x01
ret
00102$:
mov dpl,#0x03
ret
Although the dead move is eliminated in the above case, in a real firmware project I can still see a lot of cases such as:
mov r0,#_my_data1
mov a,@r0
anl a,#0x1f
mov r7,a << dead move remains
cjne a,#0x01,00102$
mov dpl,#0x00
lcall _xxxxx
mov dpl,#0x00
ret
00102$:
mov dpl,#0xff
ret
mov dptr,#(_my_data + 0x0006)
movx a,@dptr
mov r7,a << dead move remains
cjne a,#0x02,00357$
sjmp 00126$
mov dptr,#(_my_data + 0x0006)
movx a,@dptr
mov r7,a
cjne a,#0x02,00117$
push ar6
lcall _xxxxxx
pop ar6
sjmp 00126$
...
It looks this can be also reproduced, by taking the original example at the top and let it use a 16-bit comparison (caused by integer promotion of the and operation):
__data unsigned char xxx;
unsigned char bleh3 (void)
{
if ((xxx & 0x1F) == 0x10)
return 1;
else
return 3;
}
With all peepholes above included, complies to:
_bleh3:
mov r6,_xxx
anl ar6,#0x1f
mov r7,#0x00
cjne r6,#0x10,00102$
cjne r7,#0x00,00102$ << useless comparision of zero-extended high byte
mov dpl,#0x01
ret
00102$:
mov dpl,#0x03
ret
We can try to remove the useless comparison of 0 == 0
by adding the following peephole (before the duplicated peephole 302 to deal with the dead store):
replace restart {
mov %1,#0x00
cjne %2,%3,%4
cjne %1,#0x00,%4
} by {
; Peephole 800.5
mov %1,#0x00
cjne %2,%3,%4
}
It will eliminate the useless comparison, but the dead move will remain:
_bleh3:
mov r6,_xxx
anl ar6,#0x1f
mov r7,#0x00 << dead move remains
cjne r6,#0x10,00102$
mov dpl,#0x01
ret
00102$:
mov dpl,#0x03
ret
The check !checkLabelRef
in removeDeadMove
fails for some reason and it prevents it from realizing that the move is dead.
Looking at a smaller snippet of a larger function:
When the above marked
mov r6,a
is checked whether it's a dead move or not, the doTermScan / scan4op code visits labels00277
and00124
.'checkLabelRef' bails out at label
00124
because at that point (for this particular dead-move check) the label ref count = 10 and the jump-to-count is only 1, as inspected by this dead-move check. There are many other parts higher up in the function that jump to label00124
, hence its high ref count.It looks like the
checkLabelRef
check inremoveDeadMove
is superflous.In the above case we can see that the approach doesn't work as it is anticipated in the code.