[llvm-commits] [llvm] r50987 - /llvm/trunk/lib/CodeGen/MachineLICM.cpp

[Evan Cheng]

This is wrong. We definitely *want* to hoist instructions that are  
rematerializable. These instructions can be remat at the uses in case  
LICM increases register pressure and spilling ensues.

Evan

On May 12, 2008, at 12:38 PM, Bill Wendling wrote:

> Author: void
> Date: Mon May 12 14:38:32 2008
> New Revision: 50987
>
> URL: http://llvm.org/viewvc/llvm-project?rev=50987&view=rev
> Log:
> One real change - don't hoist something that's trivially  
> rematerializable. It's
> possible for it to produce worse code than before.
>
> The rest of this patch is code cleanup.
>
> Modified:
>    llvm/trunk/lib/CodeGen/MachineLICM.cpp
>
> Modified: llvm/trunk/lib/CodeGen/MachineLICM.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineLICM.cpp?rev=50987&r1=50986&r2=50987&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/lib/CodeGen/MachineLICM.cpp (original)
> +++ llvm/trunk/lib/CodeGen/MachineLICM.cpp Mon May 12 14:38:32 2008
> @@ -34,16 +34,16 @@
>   class VISIBILITY_HIDDEN MachineLICM : public MachineFunctionPass {
>     const TargetMachine   *TM;
>     const TargetInstrInfo *TII;
> -    MachineFunction       *CurMF; // Current MachineFunction
> +    MachineFunction       *CurMF;  // Current MachineFunction
>
>     // Various analyses that we use...
> -    MachineLoopInfo      *LI;   // Current MachineLoopInfo
> -    MachineDominatorTree *DT;   // Machine dominator tree for the  
> current Loop
> +    MachineLoopInfo      *LI;      // Current MachineLoopInfo
> +    MachineDominatorTree *DT;      // Machine dominator tree for  
> the cur loop
>     MachineRegisterInfo  *RegInfo; // Machine register information
>
>     // State that is updated as we process loops
> -    bool         Changed;       // True if a loop is changed.
> -    MachineLoop *CurLoop;       // The current loop we are working  
> on.
> +    bool         Changed;          // True if a loop is changed.
> +    MachineLoop *CurLoop;          // The current loop we are  
> working on.
>   public:
>     static char ID; // Pass identification, replacement for typeid
>     MachineLICM() : MachineFunctionPass((intptr_t)&ID) {}
> @@ -233,15 +233,14 @@
>     return false;
>
>   if (TID.mayLoad()) {
> -    // Okay, this instruction does a load.  As a refinement, allow  
> the target
> -    // to decide whether the loaded value is actually a constant.   
> If so, we
> -    // can actually use it as a load.
> -    if (!TII->isInvariantLoad(&I)) {
> +    // Okay, this instruction does a load. As a refinement, we  
> allow the target
> +    // to decide whether the loaded value is actually a constant.  
> If so, we can
> +    // actually use it as a load.
> +    if (!TII->isInvariantLoad(&I))
>       // FIXME: we should be able to sink loads with no other side  
> effects if
>       // there is nothing that can change memory from here until the  
> end of
> -      // block.  This is a trivial form of alias analysis.
> +      // block. This is a trivial form of alias analysis.
>       return false;
> -    }
>   }
>
>   DEBUG({
> @@ -263,12 +262,9 @@
>              *ImpDefs; ++ImpDefs)
>           DOUT << "      -> " << TRI->getName(*ImpDefs) << "\n";
>       }
> -
> -        //if (TII->hasUnmodelledSideEffects(&I))
> -        //DOUT << "  * Instruction has side effects.\n";
>     });
>
> -  // The instruction is loop invariant if all of its operands are  
> loop-invariant
> +  // The instruction is loop invariant if all of its operands are.
>   for (unsigned i = 0, e = I.getNumOperands(); i != e; ++i) {
>     const MachineOperand &MO = I.getOperand(i);
>
> @@ -282,7 +278,8 @@
>     if (TargetRegisterInfo::isPhysicalRegister(Reg))
>       return false;
>
> -    assert(RegInfo->getVRegDef(Reg)&&"Machine instr not mapped for  
> this vreg?");
> +    assert(RegInfo->getVRegDef(Reg) &&
> +           "Machine instr not mapped for this vreg?!");
>
>     // If the loop contains the definition of an operand, then the  
> instruction
>     // isn't loop invariant.
> @@ -294,12 +291,16 @@
>   return true;
> }
>
> -/// Hoist - When an instruction is found to only use loop invariant  
> operands
> -/// that is safe to hoist, this instruction is called to do the  
> dirty work.
> +/// Hoist - When an instruction is found to use only loop invariant  
> operands
> +/// that are safe to hoist, this instruction is called to do the  
> dirty work.
> ///
> void MachineLICM::Hoist(MachineInstr &MI) {
>   if (!IsLoopInvariantInst(MI)) return;
>
> +  // Hoisting things that are trivially rematerializable may result  
> in worse
> +  // code than before.
> +  if (TII->isTriviallyReMaterializable(&MI)) return;
> +
>   std::vector<MachineBasicBlock*> Preds;
>
>   // Non-back-edge predecessors.
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits