[PATCH][Review request] MachineBasicBlock::iterator bug fix

The attached patch fixes bugs related to MachineBasicBlock::iterator. I don’t have any test cases that reproduce on an X86 machine the problems that this patch fixes (these bugs were found when make check was run on a mips board).

Please review.

The first part just ensures that iterator I is not instr_end() before it invokes isInsideBundle().
The second part changes the signature of spillAll to avoid the conversion from MachineBasicBlock::iterator to MachineInstr* and back to MachineBasicBlock::iterator.

Here is the backtrace:

(gdb) bt
#0 0x2ad46fb8 in raise () from /lib/libc.so.6
#1 0x2ad4c178 in abort () from /lib/libc.so.6
#2 0x2ad3e178 in __assert_fail () from /lib/libc.so.6
#3 0x0081b60c in bundle_iterator (this=0x7fdff580, mi=0x241393c) at
#4 0x014e6f78 in spillAll (this=0x240d200, MI=0x241393c) at
#5 0x014ed6f4 in AllocateBasicBlock (this=0x240d200) at
  1. spillAll(MBB->getFirstTerminator()) at RegAllocFast.cpp:1101 is invoked. getFirstTerminator returns an end iterator here and is converted to MachineInstr* when it is passed to spillAll.
  2. spillVirtReg(MI, i) at RegAllocFast.cpp:324 is invoked. constructor of MachineBasicBlock::iterator in MachineBasicBlock.cpp:152 is called and asserts when mi->isInsideBundle returns true.

These are the signaltures of spillAll and spillVirtReg:
void spillAll(MachineInstr *MI);
void spillVirtReg(MachineBasicBlock::iterator MI, LiveRegMap::iterator);

Checking that mi is a real instruction before invoking isInsideBundle() would be a better solution, but I wasn’t sure how to go about it.

bundle_iterator(Ty *mi) : MII(mi) {
assert((!mi || !mi->isInsideBundle()) &&
“It’s not legal to initialize bundle_iterator with a bundled MI”);

isinsidebundle1.patch (1.68 KB)


Committed r167086 and r167088.

Thank you for the review.