[LLVMdev] deleting or replacing a MachineInst

[Mehdi Amini]

> On Feb 11, 2015, at 12:27 PM, Chris Sears <chris.sears at gmail.com> wrote:
> 
> 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;

The loop header needs to be modified, because MBBI will be invalidated when you remove the instruction:

    for (MachineBasicBlock::iterator MBBI = MBB->begin();
        MBBI != MBB->end(); ) {
      MachineInstr *NewMI = NULL;
      OldMI = MBBI;
      ++MBBI

The macro BUILD_INS has to be updated to use OldMI instead of MBBI.

Also I’m unsure why OldMI is not local to this loop body.


— 
Mehdi




> 
>       // %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++;
>       }
>     }
>