[llvm-commits] [llvm] r153903 - in /llvm/trunk/lib/CodeGen: InlineSpiller.cpp LiveRangeEdit.cpp LiveRangeEdit.h RegAllocBasic.cpp RegAllocGreedy.cpp RegAllocPBQP.cpp Spiller.cpp SplitKit.cpp

[Jakob Stoklund Olesen]

On Apr 2, 2012, at 3:22 PM, Pete Cooper <peter_cooper at apple.com> wrote:

> Author: pete
> Date: Mon Apr  2 17:22:53 2012
> New Revision: 153903
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=153903&view=rev
> Log:
> Refactored the LiveRangeEdit interface so that MachineFunction, TargetInstrInfo, MachineRegisterInfo, LiveIntervals, and VirtRegMap are all passed into the constructor and stored as members instead of passed in to each method.

Very nice!

> --- llvm/trunk/lib/CodeGen/LiveRangeEdit.cpp (original)
> +++ llvm/trunk/lib/CodeGen/LiveRangeEdit.cpp Mon Apr  2 17:22:53 2012
> @@ -31,13 +31,10 @@
> 
> void LiveRangeEdit::Delegate::anchor() { }
> 
> -LiveInterval &LiveRangeEdit::createFrom(unsigned OldReg,
> -                                        LiveIntervals &LIS,
> -                                        VirtRegMap &VRM) {
> -  MachineRegisterInfo &MRI = VRM.getRegInfo();
> +LiveInterval &LiveRangeEdit::createFrom(unsigned OldReg) {
>   unsigned VReg = MRI.createVirtualRegister(MRI.getRegClass(OldReg));
> -  VRM.grow();
> -  VRM.setIsSplitFromReg(VReg, VRM.getOriginal(OldReg));
> +  VRM->grow();
> +  VRM->setIsSplitFromReg(VReg, VRM->getOriginal(OldReg));

If VRM can be NULL, you should assert here, or even better just skip updating it.

> @@ -282,12 +264,14 @@
>     // Shrink just one live interval. Then delete new dead defs.
>     LiveInterval *LI = ToShrink.back();
>     ToShrink.pop_back();
> -    if (foldAsLoad(LI, Dead, MRI, LIS, TII))
> +    if (foldAsLoad(LI, Dead))
>       continue;
>     if (delegate_)
>       delegate_->LRE_WillShrinkVirtReg(LI->reg);
>     if (!LIS.shrinkToUses(LI, &Dead))
>       continue;
> +    if (!VRM)
> +      continue;

This looks suspicious. The code below does more than just updating VRM.

> @@ -94,8 +96,13 @@
>   ///                empty initially, any existing registers are ignored.
>   LiveRangeEdit(LiveInterval &parent,
>                 SmallVectorImpl<LiveInterval*> &newRegs,
> +                MachineFunction &MF,
> +                LiveIntervals &lis,
> +                VirtRegMap *vrm,
>                 Delegate *delegate = 0)

Please update the doxygen comment, and explain what happens when VRM is null.

/jakob