[llvm-commits] [llvm] r56268 - in /llvm/trunk: include/llvm/CodeGen/LinkAllCodegenComponents.h include/llvm/CodeGen/Passes.h lib/CodeGen/DeadMachineInstructionElim.cpp

[Chris Lattner]

On Sep 16, 2008, at 5:43 PM, Dan Gohman wrote:

> Author: djg
> Date: Tue Sep 16 19:43:24 2008
> New Revision: 56268
>
> URL: http://llvm.org/viewvc/llvm-project?rev=56268&view=rev
> Log:
> Add a new MachineInstr-level DCE pass. It is very simple, and is  
> intended to
> be used with fast-isel.

Ok.

> +bool  
> DeadMachineInstructionElim::runOnMachineFunction(MachineFunction  
> &MF) {
> +  bool AnyChanges = false;
> +  const MachineRegisterInfo &MRI = MF.getRegInfo();
> +  const TargetInstrInfo &TII = *MF.getTarget().getInstrInfo();
> +  bool SawStore = true;
> +
> +  // Loop over all instructions in all blocks, from bottom to top,  
> so that it's
> +  // more likely that chains of dependent but ultimately dead  
> instructions will
> +  // be cleaned up.
> +  for (MachineFunction::reverse_iterator I = MF.rbegin(), E =  
> MF.rend();
> +       I != E; ++I) {

This looks pretty nice to me.  The one big "problem" is that defs of  
physregs prevent an instruction from being removed.  Since physreg  
defs can only be local before regalloc, and since you're walking  
backward through the block already, can you just keep track of uses of  
physregs as you scan backward?

> +    MachineBasicBlock *MBB = &*I;
> +    for (MachineBasicBlock::reverse_iterator MII = MBB->rbegin(),
> +         MIE = MBB->rend(); MII != MIE; ) {
> +      MachineInstr *MI = &*MII;
> +
> +      // Don't delete instructions with side effects.
> +      if (MI->isSafeToMove(&TII, SawStore)) {

Is this really the right predicate?  Why not check explicitly for side  
effects?

Just to avoid nesting, how about:

if (!MI->isSafeToMove(&TII, SawStore)) {
   ++MII;
   continue;
}

>
> +        // Examine each operand.
> +        bool AllDefsDead = true;
> +        for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) {
> +          const MachineOperand &MO = MI->getOperand(i);
> +          if (MO.isRegister() && MO.isDef()) {
> +            unsigned Reg = MO.getReg();
> +            if (TargetRegisterInfo::isPhysicalRegister(Reg) ||
> +                !MRI.use_empty(Reg)) {
> +              // This def has a use. Don't delete the instruction!
> +              AllDefsDead = false;
> +              break;
> +            }
> +          }
> +        }

Please split this out to a helper function.

>
> +
> +        // If there are no defs with uses, the instruction is dead.
> +        if (AllDefsDead) {
> +          // Clear out the operands to take the registers out of  
> their
> +          // use chains.
> +          while (unsigned Num = MI->getNumOperands())
> +            MI->RemoveOperand(Num-1);

Why do you need to do this?  Just deleting the MI should be enough.

-Chris