The Sparc, Microblaze, and Mips code generators implement branch delay
slots. They all seem to exhibit the same bug, which is not surprising
since the code is very similar. If I compile code with this snippit:
while (n--)
*s++ = (char) c;
I get this (for the Microblaze):
swi r19, r1, 0
add r3, r0, r0
cmp r3, r3, r7
beqid r3, ($BB0_3)
brid ($BB0_1)
add r19, r1, r0
add r3, r5, r0
$BB0_2:
addi r4, r3, 1
addi r7, r7, -1
add r8, r0, r0
sbi r6, r3, 0
cmp r8, r8, r7
bneid r8, ($BB0_2)
brid ($BB0_3)
add r3, r4, r0
$BB0_3:
Notice that the label $BB0_1 is missing. If I disable filling in the
branch delay slots, I get:
swi r19, r1, 0
add r19, r1, r0
add r3, r0, r0
cmp r3, r3, r7
beqid r3, ($BB0_3)
brid ($BB0_1)
$BB0_1:
add r3, r5, r0
$BB0_2:
addi r4, r3, 1
addi r7, r7, -1
add r8, r0, r0
sbi r6, r3, 0
cmp r8, r8, r7
add r3, r4, r0
bneid r8, ($BB0_2)
brid ($BB0_3)
$BB0_3:
A similar thing happens with the Mips and Sparc, although they just fill
in the delay slots with NOPs.
My question is, why would inserting a NOP or splicing an instruction in
a MachineBasicBlock cause a label to go missing? Could someone point me
to appropriate places to look?
FYI, clang is the front end.
-Rich
On Dec 14, 2010, at 3:46 PM, Richard Pennington wrote:> Notice that the label $BB0_1 is missing. If I disable filling in the > branch delay slots, I get:Is this with the latest SVN HEAD version of LLVM or some other version? The delay slot filler and many other things have been updated for the Microblaze backend. In particular, the commit r120095 for the MBlaze backend fixed some issues with missing block labels due to AsmPrinter::isBlockOnlyReachableByFallthrough not taking into account delay slots in basic blocks.> My question is, why would inserting a NOP or splicing an instruction in > a MachineBasicBlock cause a label to go missing? Could someone point me > to appropriate places to look?The problem is that AsmPrinter::isBlockOnlyReachableByFallthrough assumes that the last instruction in a basic block is a terminator instruction, but when the terminator has a delay slot then the last instruction is not a terminator; its is a delay slot filler. This can lead AsmPrinter to think that some blocks can only be reach by a fallthrough condition even when they are branched to. When this happens it omits the block label and the assembly becomes invalid. I fixed this in the MBlaze backend by overriding AsmPrinter::isBlockOnlyReachableByFallthrough and replaced (at the end of the function): // Otherwise, check the last instruction. const MachineInstr &LastInst = Pred->back(); return !LastInst.getDesc().isBarrier(); with: // Check if the last terminator is an unconditional branch. MachineBasicBlock::const_iterator I = Pred->end(); while (I != Pred->begin() && !(--I)->getDesc().isTerminator()) ; // Noop return I == Pred->end() || !I->getDesc().isBarrier(); Notice, that the original version assumes that the last instruction is the last terminator for the basic block whereas the version in the latest Microblaze backend searches for the last terminator in the basic block. As a note, when I run the code you gave through the latest version of the Microblaze backend I do not have the problems that you describe any longer. -- Wesley Peck University of Kansas SLDG Laboratory
On 12/14/2010 04:28 PM, Wesley Peck wrote:> On Dec 14, 2010, at 3:46 PM, Richard Pennington wrote: >> Notice that the label $BB0_1 is missing. If I disable filling in the >> branch delay slots, I get: > > Is this with the latest SVN HEAD version of LLVM or some other version? The delay slot filler and many other things have been updated for the Microblaze backend. In particular, the commit r120095 for the MBlaze backend fixed some issues with missing block labels due to AsmPrinter::isBlockOnlyReachableByFallthrough not taking into account delay slots in basic blocks. >[snip] Thanks Wesley! I'm currently using r119910. I'll update. -Rich