[PATCH] MachineCSE: Add a target query for the LookAheadLimit heurisitic

[Matthias Braun]

Hi Tom,

I don't know the history here, but as this does scan forward for each instruction of the basic block it looks like a way to avoid quadratic runtime behavior for (corner) cases with thousands of instructions in a basic block. I think it is no problem to go to a much higher limit than 5. But why go completely boundless, do you need a guarantee here that the CSE is happening?

- Matthias

> On May 4, 2015, at 8:09 AM, Tom Stellard <thomas.stellard at amd.com> wrote:
> 
> Hi hfinkel, ributzka, ab, MatzeB, bkramer,
> 
> This heurisitc is used to determine whether or not to CSE physical register defs.  My understanding is that this pass will only CSE a physical register def that is within 'LookAheadLimit' instructions of a common expression.
> 
> I'm adding a target query for this because the R600 backend needs this LookAheadLimit to be MAX_UINT.
> 
> I'm not sure what the original intent of this heuristic was and why the value is 5, so another possibility would be to remove it entirely.
> 
> REPOSITORY
>  rL LLVM
> 
> http://reviews.llvm.org/D9472
> 
> Files:
>  include/llvm/Target/TargetInstrInfo.h
>  lib/CodeGen/MachineCSE.cpp
>  lib/Target/R600/SIInstrInfo.h
> 
> Index: include/llvm/Target/TargetInstrInfo.h
> ===================================================================
> --- include/llvm/Target/TargetInstrInfo.h
> +++ include/llvm/Target/TargetInstrInfo.h
> @@ -1235,6 +1235,14 @@
>     return false;
>   }
> 
> +  /// \brief Return the value to use for the MachineCSE's LookAheadLimit,
> +  /// which is a heuristic used for CSE'ing phys reg defs.
> +  virtual unsigned getMachineCSELookAheadLimit () const {
> +    // 5 is the value for this heuristic that used to be hard-coded into
> +    // the MachineCSE pass.  I have no idea why 5 was choosen.
> +    return 5;
> +  }
> +
> private:
>   int CallFrameSetupOpcode, CallFrameDestroyOpcode;
> };
> Index: lib/CodeGen/MachineCSE.cpp
> ===================================================================
> --- lib/CodeGen/MachineCSE.cpp
> +++ lib/CodeGen/MachineCSE.cpp
> @@ -48,7 +48,7 @@
>     MachineRegisterInfo *MRI;
>   public:
>     static char ID; // Pass identification
> -    MachineCSE() : MachineFunctionPass(ID), LookAheadLimit(5), CurrVN(0) {
> +    MachineCSE() : MachineFunctionPass(ID), LookAheadLimit(0), CurrVN(0) {
>       initializeMachineCSEPass(*PassRegistry::getPassRegistry());
>     }
> 
> @@ -69,7 +69,7 @@
>     }
> 
>   private:
> -    const unsigned LookAheadLimit;
> +    unsigned LookAheadLimit;
>     typedef RecyclingAllocator<BumpPtrAllocator,
>         ScopedHashTableVal<MachineInstr*, unsigned> > AllocatorTy;
>     typedef ScopedHashTable<MachineInstr*, unsigned,
> @@ -716,5 +716,6 @@
>   MRI = &MF.getRegInfo();
>   AA = &getAnalysis<AliasAnalysis>();
>   DT = &getAnalysis<MachineDominatorTree>();
> +  LookAheadLimit = TII->getMachineCSELookAheadLimit();
>   return PerformCSE(DT->getRootNode());
> }
> Index: lib/Target/R600/SIInstrInfo.h
> ===================================================================
> --- lib/Target/R600/SIInstrInfo.h
> +++ lib/Target/R600/SIInstrInfo.h
> @@ -142,6 +142,8 @@
>   bool FoldImmediate(MachineInstr *UseMI, MachineInstr *DefMI,
>                      unsigned Reg, MachineRegisterInfo *MRI) const final;
> 
> +  unsigned getMachineCSELookAheadLimit() const override { return UINT_MAX; }
> +
>   bool isSALU(uint16_t Opcode) const {
>     return get(Opcode).TSFlags & SIInstrFlags::SALU;
>   }
> 
> EMAIL PREFERENCES
>  http://reviews.llvm.org/settings/panel/emailpreferences/
> <D9472.24880.patch>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits