[llvm-commits] [llvm] r82629 - /llvm/trunk/lib/CodeGen/PostRASchedulerList.cpp

[Evan Cheng]

On Sep 23, 2009, at 10:41 PM, David Goodwin wrote:

> Because then the superreg is never killed... I think that will cause  
> an assertion in the following code when the superreg is defined again.

That means the superreg kill should be moved down to the last use of  
any of subreg. BTW, I have disabled the scavenger double-def assertion  
for now. I've found coalescer can eliminate instructions which carry  
the extra kill markers. It's not always possible to move the markers.

My long term plan is:
1. Introduce "kill" pseudo instruction that terminate liveness of  
instructions.
2. Move physical liveness computation out of livevariables and allow  
codegen passes like postalloc sched to use it.

Note double defs is not a miscomputation. It's just a performance  
issue. That's why I've decided to disable the assertion for now.

Evan

>
> David
>
> On Sep 23, 2009, at 10:34 PM, Evan Cheng wrote:
>
>>
>> On Sep 23, 2009, at 9:35 AM, David Goodwin wrote:
>>
>>> Author: david_goodwin
>>> Date: Wed Sep 23 11:35:25 2009
>>> New Revision: 82629
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=82629&view=rev
>>> Log:
>>> Fix bug in kill flag updating for post-register-allocation  
>>> scheduling. When the kill flag of a superreg needs to be cleared  
>>> because there are one or more subregs live, we instead add  
>>> implicit-defs of those subregs and leave the kill flag on the  
>>> superreg. This allows us to end the live-range of the superreg  
>>> without ending the live-ranges of the subregs.
>>>
>>
>> Hmm. This seems wrong. You have changed the code so the live  
>> registers are defined by implicit_def's. Why not remove the kill of  
>> the superreg and add kills of the sub-registers that are no longer  
>> used?
>>
>> Evan
>>
>>
>>> Modified:
>>>  llvm/trunk/lib/CodeGen/PostRASchedulerList.cpp
>>>
>>> Modified: llvm/trunk/lib/CodeGen/PostRASchedulerList.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/PostRASchedulerList.cpp?rev=82629&r1=82628&r2=82629&view=diff
>>>
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> ====================================================================
>>> --- llvm/trunk/lib/CodeGen/PostRASchedulerList.cpp (original)
>>> +++ llvm/trunk/lib/CodeGen/PostRASchedulerList.cpp Wed Sep 23  
>>> 11:35:25 2009
>>> @@ -178,6 +178,11 @@
>>>                                     unsigned LastNewReg,
>>>                                     const TargetRegisterClass *);
>>>   void StartBlockForKills(MachineBasicBlock *BB);
>>> +
>>> +    // ToggleKillFlag - Toggle a register operand kill flag. Other
>>> +    // adjustments may be made to the instruction if necessary.  
>>> Return
>>> +    // true if the operand has been deleted, false if not.
>>> +    bool ToggleKillFlag(MachineInstr *MI, MachineOperand &MO);
>>> };
>>> }
>>>
>>> @@ -822,6 +827,40 @@
>>> }
>>> }
>>>
>>> +bool SchedulePostRATDList::ToggleKillFlag(MachineInstr *MI,
>>> +                                          MachineOperand &MO) {
>>> +  // Setting kill flag...
>>> +  if (!MO.isKill()) {
>>> +    MO.setIsKill(true);
>>> +    return false;
>>> +  }
>>> +
>>> +  // If MO itself is live, clear the kill flag...
>>> +  if (KillIndices[MO.getReg()] != ~0u) {
>>> +    MO.setIsKill(false);
>>> +    return false;
>>> +  }
>>> +
>>> +  // If any subreg of MO is live, then create an imp-def for that
>>> +  // subreg and keep MO marked as killed.
>>> +  bool AllDead = true;
>>> +  const unsigned SuperReg = MO.getReg();
>>> +  for (const unsigned *Subreg = TRI->getSubRegisters(SuperReg);
>>> +       *Subreg; ++Subreg) {
>>> +    if (KillIndices[*Subreg] != ~0u) {
>>> +      MI->addOperand(MachineOperand::CreateReg(*Subreg,
>>> +                                               true  /*IsDef*/,
>>> +                                               true  /*IsImp*/,
>>> +                                               false /*IsKill*/,
>>> +                                               false /*IsDead*/));
>>> +      AllDead = false;
>>> +    }
>>> +  }
>>> +
>>> +  MO.setIsKill(AllDead);
>>> +  return false;
>>> +}
>>> +
>>> /// FixupKills - Fix the register kill flags, they may have been  
>>> made
>>> /// incorrect by instruction reordering.
>>> ///
>>> @@ -860,9 +899,9 @@
>>>     }
>>>   }
>>>
>>> -    // Examine all used registers and set kill flag. When a  
>>> register
>>> -    // is used multiple times we only set the kill flag on the  
>>> first
>>> -    // use.
>>> +    // Examine all used registers and set/clear kill flag. When a
>>> +    // register is used multiple times we only set the kill flag on
>>> +    // the first use.
>>>   killedRegs.clear();
>>>   for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) {
>>>     MachineOperand &MO = MI->getOperand(i);
>>> @@ -889,8 +928,12 @@
>>>     }
>>>
>>>     if (MO.isKill() != kill) {
>>> -        MO.setIsKill(kill);
>>> -        DEBUG(errs() << "Fixed " << MO << " in ");
>>> +        bool removed = ToggleKillFlag(MI, MO);
>>> +        if (removed) {
>>> +          DEBUG(errs() << "Fixed <removed> in ");
>>> +        } else {
>>> +          DEBUG(errs() << "Fixed " << MO << " in ");
>>> +        }
>>>       DEBUG(MI->dump());
>>>     }
>>>
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>