There are 11 BuildMI() functions in MachineInstrBuilder.h including four using the iterator and one using an instruction. But I just don't think that's it. The creation of the new instruction works fine (works fine with OldMI as well) and the new instruction is present in the assembly output. The problem is removing the old instruction correctly.> The loop header needs to be modified, because MBBI will be invalidatedwhen you remove the instruction: So if I remove the old instruction with something like: MBB->remove_instr(OldMI); I could just start the loop over or is there a more better way to invalidate the MBBI iterator? Basic blocks aren't that long (on average 4-6 instructions) and this peephole isn't that common either. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150211/25aea4a3/attachment.html>
> On Feb 11, 2015, at 2:46 PM, Chris Sears <chris.sears at gmail.com> wrote: > > There are 11 BuildMI() functions in MachineInstrBuilder.h including four using the iterator and one using an instruction. But I just don't think that's it. The creation of the new instruction works fine (works fine with OldMI as well) and the new instruction is present in the assembly output. > > The problem is removing the old instruction correctly. > > > The loop header needs to be modified, because MBBI will be invalidated when you remove the instruction: > > So if I remove the old instruction with something like: > > MBB->remove_instr(OldMI); > > I could just start the loop over or is there a more better way to invalidate the MBBI iterator? > Basic blocks aren't that long (on average 4-6 instructions) and this peephole isn't that common either. >I don’t understand anything of what you say, sorry :) I think I sent you a fix in my previous email, did you try it? Mehdi -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150211/146d3e25/attachment.html>
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>