I made the change to the BuildMI() call. Again, I don't think that matters. #define BUILD_INS(opcode, new_reg, i) \ BuildMI(*MBB, OldMI, MBBI->getDebugLoc(), TII->get(X86::opcode)) \ .addReg(X86::new_reg, kill).addImm(i) I didn't completely understand your other proposed change: for (MachineBasicBlock::iterator MBBI = MBB->begin(); MBBI != MBB->end(); ) { MachineInstr *NewMI = NULL; OldMI = MBBI; ++MBBI; I think you're saying with ++MBBI to step past the old instruction. This seems faster speedwise but more of a hack than just restarting the loop. But I'll try it. It implies a certain knowledge of the iterator and MBB. I accidentally touched MachineInstr.h and the rebuild is extracting its punishment. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150211/e328ab9e/attachment.html>
> On Feb 11, 2015, at 3:04 PM, Chris Sears <chris.sears at gmail.com> wrote: > > I made the change to the BuildMI() call. Again, I don't think that matters. > > #define BUILD_INS(opcode, new_reg, i) \ > BuildMI(*MBB, OldMI, MBBI->getDebugLoc(), TII->get(X86::opcode)) \ > .addReg(X86::new_reg, kill).addImm(i) > > I didn't completely understand your other proposed change: > > for (MachineBasicBlock::iterator MBBI = MBB->begin(); > MBBI != MBB->end(); ) { > MachineInstr *NewMI = NULL; > OldMI = MBBI; > ++MBBI; > > I think you're saying with ++MBBI to step past the old instruction. > This seems faster speedwise but more of a hack than just restarting the loop. > But I'll try it. It implies a certain knowledge of the iterator and MBB.I don’t see this is as a hack. I believe it is a widespread pattern when iterating on a linked-list and having to delete elements. You’ll see this pattern everywhere in LLVM. Mehdi -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150211/663817eb/attachment.html>
Perhaps a better more neutral term is leaky abstraction. The MBB iterator provides a documented abstraction. Look it up. Use it. Done. Do not worry about how it works. Then along comes a problem which isn't within that abstraction's abilities. So people come up with a solution/workaround/xxxx which then becomes an accepted pattern and they use that. This works. The trouble is when other people who just read the documentation expect an abstraction and its methods to just work. And they can't figure out why. We don't really want to understand the implementation and search for patterns, albeit widespread. It should be in the abstraction. It should be in the documentation. Now of course, I'm going to use this approach because it is faster than restarting the loop. However, if it's so widespread and it does indeed work, why not add it to the abstraction? Existing code will continue to work but then this will be easier the next time for newbies to figure out in the future. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150212/518a4880/attachment.html>