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