[llvm-commits] [llvm] r112741 - /llvm/trunk/lib/CodeGen/RegAllocLinearScan.cpp

[Jim Grosbach]

On Sep 10, 2010, at 8:45 AM, Jakob Stoklund Olesen wrote:

> 
> On Sep 9, 2010, at 11:49 PM, Evan Cheng wrote:
> 
>> To me it seems like tri_->getAllocatableSet(fn), allocation_order_begin, allocation_order_end should not include reserved registers. After all, their names implied as such.
> 
> I agree in principle, and tri_->getAllocatableSet(fn) has since been fixed to filter out the reserved regs.
> 
> Only the iterators may include reserved regs.

Right. The idea is that getAllocatableSet() should become the canonical way of determining which registers are allocatable. Allocation orders are a lower level interface that say, in effect, "Of those registers that may be allocatable, what order should they be allocated in." That is, they include all registers that might be allocatable, and any entry is checked against the allocatable set before use. This allows things like the frame pointer being allocatable (or not) on a function by function basis without the allocation order iterators needing to change.


> 
>> Wouldn't every register allocator needs to do the same check?
> 
> Yes. We are missing a comment in TargetRegisterInfo.h to that effect.
> 

Good point. I'll do something about that.

> I agree that it isn't pretty, but it is an improvement over the old system. The allocation_order_* methods in the RegisterInfo.td files were getting too complicated and error prone.
> 
> The system worked well for the occasional reserved frame pointer on x86, but the number of registers that can be reserved on ARM make the combinatorics untenable.
> 
> Ultimately, I would like to see tablegen getting more involved in this. Tablegen should know which registers may be reserved and the corresponding predicates. Then it can generate allocation_order_* methods based on that

Absolutely. There's a lot of room for improvement here. We should be able get to a solution that doesn't require targets to directly implement allocation order iterators at all, at least for typical use cases. They should be created automatically based on predicates expressed in the td files. This will be a huge improvement, as we'll be moving towards allowing back-ends to express intent for the register classes rather than having to specify details of implementation for the register allocator(s).

For example, given the register classes, the calling convention(s), the isReservedReg() hook and the subtarget predicates, it should be entirely reasonable to auto-generate the necessary register allocation orders from the register classes without the target having to do anything else barring something really weird. We're not there yet, obviously, but we're getting there.

-Jim


> 
>> 
>> Evan
>> 
>> On Sep 1, 2010, at 2:04 PM, Jim Grosbach wrote:
>> 
>>> Author: grosbach
>>> Date: Wed Sep  1 16:04:27 2010
>>> New Revision: 112741
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=112741&view=rev
>>> Log:
>>> The register allocator shouldn't consider allocating reserved registers.
>>> r112728 did this for fast regalloc.
>>> 
>>> Modified:
>>>  llvm/trunk/lib/CodeGen/RegAllocLinearScan.cpp
>>> 
>>> Modified: llvm/trunk/lib/CodeGen/RegAllocLinearScan.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/RegAllocLinearScan.cpp?rev=112741&r1=112740&r2=112741&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/CodeGen/RegAllocLinearScan.cpp (original)
>>> +++ llvm/trunk/lib/CodeGen/RegAllocLinearScan.cpp Wed Sep  1 16:04:27 2010
>>> @@ -125,6 +125,7 @@
>>>   const TargetRegisterInfo* tri_;
>>>   const TargetInstrInfo* tii_;
>>>   BitVector allocatableRegs_;
>>> +    BitVector reservedRegs_;
>>>   LiveIntervals* li_;
>>>   LiveStacks* ls_;
>>>   MachineLoopInfo *loopInfo;
>>> @@ -464,6 +465,7 @@
>>> tri_ = tm_->getRegisterInfo();
>>> tii_ = tm_->getInstrInfo();
>>> allocatableRegs_ = tri_->getAllocatableSet(fn);
>>> +  reservedRegs_ = tri_->getReservedRegs(fn);
>>> li_ = &getAnalysis<LiveIntervals>();
>>> ls_ = &getAnalysis<LiveStacks>();
>>> loopInfo = &getAnalysis<MachineLoopInfo>();
>>> @@ -949,8 +951,14 @@
>>> const TargetRegisterClass *RC = mri_->getRegClass(cur->reg);
>>> if (cur->empty()) {
>>>   unsigned physReg = vrm_->getRegAllocPref(cur->reg);
>>> -    if (!physReg)
>>> -      physReg = *RC->allocation_order_begin(*mf_);
>>> +    if (!physReg) {
>>> +      TargetRegisterClass::iterator aoe = RC->allocation_order_end(*mf_);
>>> +      TargetRegisterClass::iterator i = RC->allocation_order_begin(*mf_);
>>> +      while (reservedRegs_.test(*i) && i != aoe)
>>> +        ++i;
>>> +      assert(i != aoe && "All registers reserved?!");
>>> +      physReg = *i;
>>> +    }
>>>   DEBUG(dbgs() <<  tri_->getName(physReg) << '\n');
>>>   // Note the register is not really in use.
>>>   vrm_->assignVirt2Phys(cur->reg, physReg);
>>> @@ -1133,8 +1141,9 @@
>>>          e = RC->allocation_order_end(*mf_); i != e; ++i) {
>>>     unsigned reg = *i;
>>>     float regWeight = SpillWeights[reg];
>>> -      // Skip recently allocated registers.
>>> -      if (minWeight > regWeight && !isRecentlyUsed(reg))
>>> +      // Skip recently allocated registers and reserved registers.
>>> +      if (minWeight > regWeight && !isRecentlyUsed(reg) &&
>>> +          !reservedRegs_.test(reg))
>>>       Found = true;
>>>     RegsWeights.push_back(std::make_pair(reg, regWeight));
>>>   }
>>> @@ -1144,6 +1153,8 @@
>>>   for (TargetRegisterClass::iterator i = RC->allocation_order_begin(*mf_),
>>>          e = RC->allocation_order_end(*mf_); i != e; ++i) {
>>>     unsigned reg = *i;
>>> +      if (reservedRegs_.test(reg))
>>> +        continue;
>>>     // No need to worry about if the alias register size < regsize of RC.
>>>     // We are going to spill all registers that alias it anyway.
>>>     for (const unsigned* as = tri_->getAliasSet(reg); *as; ++as)
>>> @@ -1157,7 +1168,15 @@
>>> minWeight = RegsWeights[0].second;
>>> if (minWeight == HUGE_VALF) {
>>>   // All registers must have inf weight. Just grab one!
>>> -    minReg = BestPhysReg ? BestPhysReg : *RC->allocation_order_begin(*mf_);
>>> +    if (BestPhysReg == 0) {
>>> +      TargetRegisterClass::iterator aoe = RC->allocation_order_end(*mf_);
>>> +      TargetRegisterClass::iterator i = RC->allocation_order_begin(*mf_);
>>> +      while (reservedRegs_.test(*i) && i != aoe)
>>> +        ++i;
>>> +      assert(i != aoe && "All registers reserved?!");
>>> +      minReg = *i;
>>> +    } else
>>> +      minReg = BestPhysReg;
>>>   if (cur->weight == HUGE_VALF ||
>>>       li_->getApproximateInstructionCount(*cur) == 0) {
>>>     // Spill a physical register around defs and uses.
>>> @@ -1414,6 +1433,9 @@
>>>   // Ignore "downgraded" registers.
>>>   if (SkipDGRegs && DowngradedRegs.count(Reg))
>>>     continue;
>>> +    // Skip reserved registers.
>>> +    if (reservedRegs_.test(Reg))
>>> +      continue;
>>>   // Skip recently allocated registers.
>>>   if (isRegAvail(Reg) && !isRecentlyUsed(Reg)) {
>>>     FreeReg = Reg;
>>> @@ -1442,6 +1464,9 @@
>>>   // Ignore "downgraded" registers.
>>>   if (SkipDGRegs && DowngradedRegs.count(Reg))
>>>     continue;
>>> +    // Skip reserved registers.
>>> +    if (reservedRegs_.test(Reg))
>>> +      continue;
>>>   if (isRegAvail(Reg) && Reg < inactiveCounts.size() &&
>>>       FreeRegInactiveCount < inactiveCounts[Reg] && !isRecentlyUsed(Reg)) {
>>>     FreeReg = Reg;
>>> 
>>> 
>>> _______________________________________________
>>> 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
>