[llvm-commits] [llvm] r149144 - /llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp

[Lang Hames]

Yeah - not sure how I missed that. Thanks for catching it.

Backed out in r149146 . Will look for a neater way to refactor this.

- Lang.

On Jan 27, 2012, at 1:33 PM, Benjamin Kramer wrote:

> 
> On 27.01.2012, at 20:58, Lang Hames wrote:
> 
>> Author: lhames
>> Date: Fri Jan 27 13:58:14 2012
>> New Revision: 149144
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=149144&view=rev
>> Log:
>> Move some duplicate loops in the coalescer into their own function.
>> 
>> Modified:
>>   llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp
>> 
>> Modified: llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp?rev=149144&r1=149143&r2=149144&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp Fri Jan 27 13:58:14 2012
>> @@ -1421,6 +1421,42 @@
>>  return true;
>> }
>> 
>> +// Loop over the value numbers of the Dst interval and record the values that
>> +// are defined by a copy from the Src interval.
>> +static void FindValuesCopiedFrom(
>> +                              LiveIntervals& lis,
>> +                              const TargetRegisterInfo& tri,
>> +                              CoalescerPair& CP, 
>> +                              LiveInterval &Dst, LiveInterval &Src,
>> +                              DenseMap<VNInfo*, VNInfo*>& DstValsDefinedFromSrc,
>> +                              SmallVector<MachineInstr*, 8>& DupCopies) {
>> +
>> +  for (LiveInterval::vni_iterator i = Dst.vni_begin(), e = Dst.vni_end();
>> +       i != e; ++i) {
>> +    VNInfo *VNI = *i;
>> +    if (VNI->isUnused() || !VNI->isDefByCopy())  // Src not defined by a copy?
>> +      continue;
>> +
>> +    // Never join with a register that has EarlyClobber redefs.
>> +    if (VNI->hasRedefByEC())
>> +      return false;
> 
> GCC complains (correctly) about returning a value from a void function.
> 
> Also you copied this early exit from the other function, effectively changing behavior
> because the caller doesn't return anymore when this condition is met.
> 
> - Ben
> 
>> +
>> +    // Figure out the value # from the Src.
>> +    LiveRange *lr = Src.getLiveRangeContaining(VNI->def.getPrevSlot());
>> +    // The copy could be to an aliased physreg.
>> +    if (!lr) continue;
>> +
>> +    // DstReg is known to be a register in the Dst interval.  If the src is
>> +    // from the Src interval, we can use its value #.
>> +    MachineInstr *MI = VNI->getCopy();
>> +    if (!CP.isCoalescable(MI) &&
>> +        !RegistersDefinedFromSameValue(lis, tri, CP, VNI, lr, DupCopies))
>> +      continue;
>> +
>> +    DstValsDefinedFromSrc[VNI] = lr->valno;
>> +  }
>> +}                            
>> +
>> /// JoinIntervals - Attempt to join these two intervals.  On failure, this
>> /// returns false.
>> bool RegisterCoalescer::JoinIntervals(CoalescerPair &CP) {
>> @@ -1506,59 +1542,9 @@
>>  LiveInterval &LHS = LIS->getOrCreateInterval(CP.getDstReg());
>>  DEBUG({ dbgs() << "\t\tLHS = "; LHS.print(dbgs(), TRI); dbgs() << "\n"; });
>> 
>> -  // Loop over the value numbers of the LHS, seeing if any are defined from
>> -  // the RHS.
>> -  for (LiveInterval::vni_iterator i = LHS.vni_begin(), e = LHS.vni_end();
>> -       i != e; ++i) {
>> -    VNInfo *VNI = *i;
>> -    if (VNI->isUnused() || !VNI->isDefByCopy())  // Src not defined by a copy?
>> -      continue;
>> -
>> -    // Never join with a register that has EarlyClobber redefs.
>> -    if (VNI->hasRedefByEC())
>> -      return false;
>> -
>> -    // Figure out the value # from the RHS.
>> -    LiveRange *lr = RHS.getLiveRangeContaining(VNI->def.getPrevSlot());
>> -    // The copy could be to an aliased physreg.
>> -    if (!lr) continue;
>> -
>> -    // DstReg is known to be a register in the LHS interval.  If the src is
>> -    // from the RHS interval, we can use its value #.
>> -    MachineInstr *MI = VNI->getCopy();
>> -    if (!CP.isCoalescable(MI) &&
>> -        !RegistersDefinedFromSameValue(*LIS, *TRI, CP, VNI, lr, DupCopies))
>> -      continue;
>> -
>> -    LHSValsDefinedFromRHS[VNI] = lr->valno;
>> -  }
>> -
>> -  // Loop over the value numbers of the RHS, seeing if any are defined from
>> -  // the LHS.
>> -  for (LiveInterval::vni_iterator i = RHS.vni_begin(), e = RHS.vni_end();
>> -       i != e; ++i) {
>> -    VNInfo *VNI = *i;
>> -    if (VNI->isUnused() || !VNI->isDefByCopy())  // Src not defined by a copy?
>> -      continue;
>> -
>> -    // Never join with a register that has EarlyClobber redefs.
>> -    if (VNI->hasRedefByEC())
>> -      return false;
>> -
>> -    // Figure out the value # from the LHS.
>> -    LiveRange *lr = LHS.getLiveRangeContaining(VNI->def.getPrevSlot());
>> -    // The copy could be to an aliased physreg.
>> -    if (!lr) continue;
>> -
>> -    // DstReg is known to be a register in the RHS interval.  If the src is
>> -    // from the LHS interval, we can use its value #.
>> -    MachineInstr *MI = VNI->getCopy();
>> -    if (!CP.isCoalescable(MI) &&
>> -        !RegistersDefinedFromSameValue(*LIS, *TRI, CP, VNI, lr, DupCopies))
>> -        continue;
>> -
>> -    RHSValsDefinedFromLHS[VNI] = lr->valno;
>> -  }
>> +  // Build a map of LHS values defined by copies from RHS and vice-versa.
>> +  FindValuesCopiedFrom(*LIS, *TRI, CP, LHS, RHS, LHSValsDefinedFromRHS, DupCopies);
>> +  FindValuesCopiedFrom(*LIS, *TRI, CP, RHS, LHS, RHSValsDefinedFromLHS, DupCopies);
>> 
>>  LHSValNoAssignments.resize(LHS.getNumValNums(), -1);
>>  RHSValNoAssignments.resize(RHS.getNumValNums(), -1);
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits