I'm writing a peephole pass and I'm done with the X86_64 instruction level detail work. But I'm having difficulty with the basic block surgery of replacing the old MachineInst. The peephole pass gets called per MachineFunction and then iterates over each MachineBasicBlock and in turn over each MachineInst. When it finds an instruction which should be replaced, it builds a new instruction: NewMI = BuildMI(*MBB, MBBI, MBBI->getDebugLoc(), TII->get(X86::opcode)) .addReg(X86::new_reg, kill) .addImm(i); This works and it correctly places the new instruction just before the old instruction in the assembly output. So far so good. Now I have to remove the old instruction. But everything I try crashes LLVM, either immediately or eventually. Various incantations which haven't worked. MBB->remove_instr(OldMI); OldMI->removeFromParent(); MBB.erase(OldMI); I should add that there are flags // %EFLAGS<imp-def> is getting copied automatically // %RDX<imp-use,kill> is not getting copied (when it appears) So, any advice to peephole adding/deleting or just replacing MachineInst's ? I've looked over the other peephole optimizers. None seem to direct replacement. You'd think there'd be a MachineInst method. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150210/b7b2cb98/attachment.html>
> On Feb 10, 2015, at 9:25 PM, Chris Sears <chris.sears at gmail.com> wrote: > > I'm writing a peephole pass and I'm done with the X86_64 instruction level detail work. But I'm having difficulty with the basic block surgery of replacing the old MachineInst. > > The peephole pass gets called per MachineFunction and then iterates over each MachineBasicBlock and in turn over each MachineInst. When it finds an instruction which should be replaced, it builds a new instruction: > > NewMI = BuildMI(*MBB, MBBI, MBBI->getDebugLoc(), TII->get(X86::opcode)) > .addReg(X86::new_reg, kill) > .addImm(i); > > This works and it correctly places the new instruction just before the old instruction in the assembly output. So far so good. > > Now I have to remove the old instruction. But everything I try crashes LLVM, either immediately or eventually. Various incantations which haven't worked.Don’t you have a problem of iterator invalidation? How do you loop through the block? If you can post the full body of the loop (if not too large), or a simpler version that has the bug. — Mehdi> > MBB->remove_instr(OldMI); > OldMI->removeFromParent(); > MBB.erase(OldMI); > > I should add that there are flags > > // %EFLAGS<imp-def> is getting copied automatically > // %RDX<imp-use,kill> is not getting copied (when it appears) > > So, any advice to peephole adding/deleting or just replacing MachineInst's ? I've looked over the other peephole optimizers. None seem to direct replacement. You'd think there'd be a MachineInst method. > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
This seems a very natural approach but I probably am having a trouble with the iterator invalidation. However, looking at other peephole optimizers passes, I couldn't see how to do this: #define BUILD_INS(opcode, new_reg, i) \ BuildMI(*MBB, MBBI, MBBI->getDebugLoc(), TII->get(X86::opcode)) \ .addReg(X86::new_reg, kill).addImm(i) for (MachineFunction::iterator MFI = MF.begin(), MFE = MF.end(); MFI != MFE; ++MFI) { MachineBasicBlock* MBB = MFI; for (MachineBasicBlock::iterator MBBI = MBB->begin(); MBBI != MBB->end(); ++MBBI) { MachineInstr *NewMI = NULL; OldMI = MBBI; // %EFLAGS<imp-def> is getting copied // %RDX<imp-use,kill> is not getting copied (when it appears) switch (OldMI->getOpcode()) { default: continue; // .... case X86::BT64ri8: case X86::BT32ri8: case X86::BT16ri8: { assert(OldMI->getNumOperands() >= 2); MachineOperand &Reg = OldMI->getOperand(0); MachineOperand &Imm = OldMI->getOperand(1); assert(Reg.isReg()); assert(Imm.isImm()); imm = Imm.getImm(); if (imm >= 32) continue; kill = getKillRegState(Reg.isKill()); switch (Reg.getReg()) { default: assert(false); case X86::RAX: case X86::EAX: case X86::AX: if (imm < 8) NewMI = BUILD_INS(TEST8i8, AL, 1 << imm); else if (imm < 16) NewMI = BUILD_INS(TEST8ri, AH, 1 << (imm - 8)); else NewMI = BUILD_INS(BT32ri8, EAX, imm); break; // ... } } break; } // NewMI has been inserted before OldMI if (NewMI != NULL) { // MBB->remove_instr(OldMI); // I've tried these (and others) // OldMI->removeFromParent(); // MBB.erase(OldMI); NumX86Peepholes++; } } -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150211/ffb87e4c/attachment.html>