[llvm-commits] [llvm] r86322 - in /llvm/trunk: include/llvm/CodeGen/LiveIntervalAnalysis.h lib/CodeGen/LiveIntervalAnalysis.cpp

[Evan Cheng]

Sorry, Jakob. I don't think this patch is right. The check is whether  
the early coalescer should be handling the cases where the normal  
coalescer cannot.

Perhaps hacking on the early coalescer is not the right approach?  
We're probably better off starting over by adding on-demand critical  
edge splitting in phi elimination.

Evan

On Nov 6, 2009, at 5:58 PM, Jakob Stoklund Olesen wrote:

> Author: stoklund
> Date: Fri Nov  6 19:58:40 2009
> New Revision: 86322
>
> URL: http://llvm.org/viewvc/llvm-project?rev=86322&view=rev
> Log:
> Fix inverted conflict test in -early-coalesce.
>
> A non-identity copy cannot be coalesced when the phi join  
> destination register
> is live at the copy site.
>
> Also verify the condition that the PHI join source register is only  
> used in
> the PHI join. Otherwise the coalescing is invalid.
>
> Modified:
>    llvm/trunk/include/llvm/CodeGen/LiveIntervalAnalysis.h
>    llvm/trunk/lib/CodeGen/LiveIntervalAnalysis.cpp
>
> Modified: llvm/trunk/include/llvm/CodeGen/LiveIntervalAnalysis.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/LiveIntervalAnalysis.h?rev=86322&r1=86321&r2=86322&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/include/llvm/CodeGen/LiveIntervalAnalysis.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/LiveIntervalAnalysis.h Fri Nov   
> 6 19:58:40 2009
> @@ -290,9 +290,10 @@
>     /// computeIntervals - Compute live intervals.
>     void computeIntervals();
>
> -    bool isProfitableToCoalesce(LiveInterval &DstInt, LiveInterval  
> &SrcInt,
> -                                SmallVector<MachineInstr*,16>  
> &IdentCopies,
> -                                SmallVector<MachineInstr*,16>  
> &OtherCopies);
> +    bool isSafeAndProfitableToCoalesce(LiveInterval &DstInt,
> +                                       LiveInterval &SrcInt,
> +                 SmallVector<MachineInstr*,16> &IdentCopies,
> +                 SmallVector<MachineInstr*,16> &OtherCopies);
>
>     void performEarlyCoalescing();
>
>
> Modified: llvm/trunk/lib/CodeGen/LiveIntervalAnalysis.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/LiveIntervalAnalysis.cpp?rev=86322&r1=86321&r2=86322&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/lib/CodeGen/LiveIntervalAnalysis.cpp (original)
> +++ llvm/trunk/lib/CodeGen/LiveIntervalAnalysis.cpp Fri Nov  6  
> 19:58:40 2009
> @@ -646,17 +646,17 @@
>                           0, false, VNInfoAllocator);
>   vni->setIsPHIDef(true);
>   LiveRange LR(start, end, vni);
> -
> +
>   interval.addRange(LR);
>   LR.valno->addKill(end);
>   DEBUG(errs() << " +" << LR << '\n');
> }
>
> -bool
> -LiveIntervals::isProfitableToCoalesce(LiveInterval &DstInt,  
> LiveInterval &SrcInt,
> -                                   SmallVector<MachineInstr*,16>  
> &IdentCopies,
> -                                   SmallVector<MachineInstr*,16>  
> &OtherCopies) {
> -  bool HaveConflict = false;
> +bool LiveIntervals::
> +isSafeAndProfitableToCoalesce(LiveInterval &DstInt,
> +                              LiveInterval &SrcInt,
> +                              SmallVector<MachineInstr*,16>  
> &IdentCopies,
> +                              SmallVector<MachineInstr*,16>  
> &OtherCopies) {
>   unsigned NumIdent = 0;
>   for (MachineRegisterInfo::def_iterator ri = mri_->def_begin 
> (SrcInt.reg),
>          re = mri_->def_end(); ri != re; ++ri) {
> @@ -665,16 +665,16 @@
>     if (!tii_->isMoveInstr(*MI, SrcReg, DstReg, SrcSubReg, DstSubReg))
>       return false;
>     if (SrcReg != DstInt.reg) {
> +      // Non-identity copy - we cannot handle overlapping intervals
> +      if (DstInt.liveAt(getInstructionIndex(MI)))
> +        return false;
>       OtherCopies.push_back(MI);
> -      HaveConflict |= DstInt.liveAt(getInstructionIndex(MI));
>     } else {
>       IdentCopies.push_back(MI);
>       ++NumIdent;
>     }
>   }
>
> -  if (!HaveConflict)
> -    return false; // Let coalescer handle it
>   return IdentCopies.size() > OtherCopies.size();
> }
>
> @@ -701,19 +701,21 @@
>     LiveInterval &SrcInt = getInterval(PHISrc);
>     SmallVector<MachineInstr*, 16> IdentCopies;
>     SmallVector<MachineInstr*, 16> OtherCopies;
> -    if (!isProfitableToCoalesce(DstInt, SrcInt, IdentCopies,  
> OtherCopies))
> +    if (!isSafeAndProfitableToCoalesce(DstInt, SrcInt,
> +                                       IdentCopies, OtherCopies))
>       continue;
>
>     DEBUG(errs() << "PHI Join: " << *Join);
>     assert(DstInt.containsOneValue() && "PHI join should have just  
> one val#!");
> +    assert(std::distance(mri_->use_begin(PHISrc), mri_->use_end())  
> == 1 &&
> +           "PHI join src should not be used elsewhere");
>     VNInfo *VNI = DstInt.getValNumInfo(0);
>
>     // Change the non-identity copies to directly target the phi  
> destination.
>     for (unsigned i = 0, e = OtherCopies.size(); i != e; ++i) {
>       MachineInstr *PHICopy = OtherCopies[i];
> -      DEBUG(errs() << "Moving: " << *PHICopy);
> -
>       SlotIndex MIIndex = getInstructionIndex(PHICopy);
> +      DEBUG(errs() << "Moving: " << MIIndex << ' ' << *PHICopy);
>       SlotIndex DefIndex = MIIndex.getDefIndex();
>       LiveRange *SLR = SrcInt.getLiveRangeContaining(DefIndex);
>       SlotIndex StartIndex = SLR->start;
> @@ -724,8 +726,7 @@
>       SrcInt.removeValNo(SLR->valno);
>       DEBUG(errs() << "  added range [" << StartIndex << ','
>             << EndIndex << "] to reg" << DstInt.reg << '\n');
> -      if (DstInt.liveAt(StartIndex))
> -        DstInt.removeRange(StartIndex, EndIndex);
> +      assert (!DstInt.liveAt(StartIndex) && "Cannot coalesce when  
> dst live!");
>       VNInfo *NewVNI = DstInt.getNextValue(DefIndex, PHICopy, true,
>                                            VNInfoAllocator);
>       NewVNI->setHasPHIKill(true);
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits