[llvm-commits] [llvm] r51562 - /llvm/trunk/lib/CodeGen/TwoAddressInstructionPass.cpp

[Evan Cheng]

On May 25, 2008, at 10:18 PM, Bill Wendling wrote:

> Author: void
> Date: Mon May 26 00:18:34 2008
> New Revision: 51562
>
> URL: http://llvm.org/viewvc/llvm-project?rev=51562&view=rev
> Log:
> A problem that's exposed when machine LICM is enabled. Consider this  
> code:
>
> LBB1_3:   # bb
> ..
>        xorl    %ebp, %ebp
>        subl    (%ebx), %ebp
> ..
>        incl    %ecx
>        cmpl    %edi, %ecx
>        jl      LBB1_3  # bb
>
> Whe using machine LICM, LLVM converts it into:
>
>        xorl %esi, %esi
> LBB1_3: # bb
> ..
>        movl    %esi, %ebp
>        subl    (%ebx), %ebp
> ..
>        incl    %ecx
>        cmpl    %edi, %ecx
>        jl      LBB1_3  # bb
>
> Two address conversion inserts the copy instruction. However, it's  
> cheaper to
> rematerialize it, and remat helps reduce register pressure.
>
> Modified:
>    llvm/trunk/lib/CodeGen/TwoAddressInstructionPass.cpp
>
> Modified: llvm/trunk/lib/CodeGen/TwoAddressInstructionPass.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/TwoAddressInstructionPass.cpp?rev=51562&r1=51561&r2=51562&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/lib/CodeGen/TwoAddressInstructionPass.cpp (original)
> +++ llvm/trunk/lib/CodeGen/TwoAddressInstructionPass.cpp Mon May 26  
> 00:18:34 2008
> @@ -39,6 +39,7 @@
> #include "llvm/Target/TargetMachine.h"
> #include "llvm/Support/Compiler.h"
> #include "llvm/Support/Debug.h"
> +#include "llvm/ADT/SmallPtrSet.h"
> #include "llvm/ADT/Statistic.h"
> #include "llvm/ADT/STLExtras.h"
> using namespace llvm;
> @@ -200,6 +201,8 @@
>   DOUT << "********** REWRITING TWO-ADDR INSTRS **********\n";
>   DOUT << "********** Function: " << MF.getFunction()->getName() <<  
> '\n';
>
> +  SmallPtrSet<MachineInstr*, 8> ReMattedInstrs;
> +
>   for (MachineFunction::iterator mbbi = MF.begin(), mbbe = MF.end();
>        mbbi != mbbe; ++mbbi) {
>     for (MachineBasicBlock::iterator mi = mbbi->begin(), me = mbbi- 
> >end();
> @@ -321,7 +324,14 @@
>
>         InstructionRearranged:
>           const TargetRegisterClass* rc =  
> MF.getRegInfo().getRegClass(regA);
> -          TII->copyRegToReg(*mbbi, mi, regA, regB, rc, rc);
> +          MachineInstr *Orig = MRI->getVRegDef(regB);
> +
> +          if (Orig && TII->isTriviallyReMaterializable(Orig)) {
> +            TII->reMaterialize(*mbbi, mi, regA, Orig);
> +            ReMattedInstrs.insert(Orig);

Do you see any performance impact from this patch? It's not clear this  
is always a good idea. It's almost always a good idea to remat instead  
of spill, but not necessarily the case here. I wonder if this needs to  
check if the instruction is "cheap"?

>
> +          } else {
> +            TII->copyRegToReg(*mbbi, mi, regA, regB, rc, rc);
> +          }
>
>           MachineBasicBlock::iterator prevMi = prior(mi);
>           DOUT << "\t\tprepend:\t"; DEBUG(prevMi- 
> >print(*cerr.stream(), &TM));
> @@ -357,5 +367,34 @@
>     }
>   }
>
> +  SmallPtrSet<MachineInstr*, 8>::iterator I = ReMattedInstrs.begin();
> +  SmallPtrSet<MachineInstr*, 8>::iterator E = ReMattedInstrs.end();
> +
> +  for (; I != E; ++I) {
> +    MachineInstr *MI = *I;
> +    bool InstrDead = true;
> +
> +    for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) {
> +      const MachineOperand &MO = MI->getOperand(i);
> +      if (!MO.isRegister())
> +        continue;
> +      unsigned MOReg = MO.getReg();
> +      if (!MOReg)
> +        continue;
> +      if (MO.isDef()) {
> +        if (MO.isImplicit())
> +          continue;

How about?

        is (!MOReg || !MO.isDef() || MO.isImplicit())
          continue;

>
> +
> +        if (MRI->use_begin(MOReg) != MRI->use_end()) {
> +          InstrDead = false;
> +          break;
> +        }
> +      }
> +    }
> +
> +    if (InstrDead && MI->getNumOperands() > 0)

Why check for MI->getNumOperands() > 0?

Evan

>
> +      MI->eraseFromParent();
> +  }
> +
>   return MadeChange;
> }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits