llvm dev - Oct 2012 - [LLVMdev] [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
/scratch/tmp/octeon/llvm-test/source/include/llvm/CodeGen/MachineBasicBlock.h:153
#4 0x014e6f78 in spillAll (this=0x240d200, MI=0x241393c) at
/scratch/tmp/octeon/llvm-test/source/lib/CodeGen/RegAllocFast.cpp:324
#5 0x014ed6f4 in AllocateBasicBlock (this=0x240d200) at
/scratch/tmp/octeon/llvm-test/source/lib/CodeGen/RegAllocFast.cpp:1101


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");
}
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20121030/e5ae540a/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: isinsidebundle1.patch
Type: application/octet-stream
Size: 1725 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20121030/e5ae540a/attachment.obj>

On Oct 30, 2012, at 11:57 AM, Akira Hatanaka <ahatanak at gmail.com>
wrote:
> 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.
LGTM.

Committed r167086 and r167088.

Thank you for the review.

On Tue, Oct 30, 2012 at 1:02 PM, Jakob Stoklund Olesen <stoklund at
2pi.dk>wrote:
>
> On Oct 30, 2012, at 11:57 AM, Akira Hatanaka <ahatanak at gmail.com>
wrote:
>
> > 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.
>
> LGTM.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20121030/2de7334c/attachment.html>