llvm dev - Feb 2015 - [LLVMdev] deleting or replacing a MachineInst

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>