[llvm] r192623 - LiveRegUnits::removeRegsInMask safety.

[Andrew Trick]

On Oct 14, 2013, at 3:51 PM, Eric Christopher <echristo at gmail.com> wrote:

> Can you give an example here? The best I could come up with was
> something where you say you're clobbering all of a super register, but
> then have a mask that says you're only clobbering part. That seems...
> wonky or inefficient or just plain wrong - an example would go a long
> way here.

I’ll try. I mentioned this potential problem in my (post-commit) review when the code was checked in last week.

On Win64 these registers are callee-save: XMM6-XMM15. So this is totally legit:

XMM6 = VADDSDrr …
<YMM6 dead, XMM6 live>
CALL … <regmask preserves: XMM6-15>
<YMM0 dead, XMM6 live>b
XMM0 = VMULSDrr XMM6, …

The call’s regmask does not preserve YMM6. We have only one register unit for XMM6 and YMM6. But when we see that YMM6 is clobbered, we cannot assume the XMM6 is also dead above the call. 

Jakob originally observed this problem and added a helpful comment in LiveRegMatrix::checkRegMaskInterference. My interpretation of the problem is that liveness should mark a register unit dead only when all superregisters are clobbered.

The situation is different when we see a definition of register (XMM0=VMULSDrr in the above example). In this case, we must be allowed to assume that an instruction sets the high bits of the register and consequently clobbers YMM0. If we wanted to preserve YMM0, we would need to define a register unit for the high bits, or add an <imp-use> operand to the instruction.

If we wanted to track dead registers instead of live registers to be more precise, I think we would need to track each physical register number instead. Register units would not be much use.

-Andy


> On Mon, Oct 14, 2013 at 1:45 PM, Andrew Trick <atrick at apple.com> wrote:
>> Author: atrick
>> Date: Mon Oct 14 15:45:19 2013
>> New Revision: 192623
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=192623&view=rev
>> Log:
>> LiveRegUnits::removeRegsInMask safety.
>> 
>> Clobbering is exclusive not inclusive on register units.
>> For liveness, we need to consider all the preserved registers.
>> e.g. A regmask that clobbers YMM0 may preserve XMM0.
>> Units are only clobbered when all super-registers are clobbered.
>> 
>> Modified:
>>    llvm/trunk/lib/CodeGen/LiveRegUnits.cpp
>> 
>> Modified: llvm/trunk/lib/CodeGen/LiveRegUnits.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/LiveRegUnits.cpp?rev=192623&r1=192622&r2=192623&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/LiveRegUnits.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/LiveRegUnits.cpp Mon Oct 14 15:45:19 2013
>> @@ -17,21 +17,30 @@
>> #include "llvm/CodeGen/MachineInstrBundle.h"
>> using namespace llvm;
>> 
>> +/// Return true if the given MachineOperand clobbers the given register unit.
>> +/// A register unit is only clobbered if all its super-registers are clobbered.
>> +static bool operClobbersUnit(const MachineOperand *MO, unsigned Unit,
>> +                             const MCRegisterInfo *MCRI) {
>> +  for (MCRegUnitRootIterator RI(Unit, MCRI); RI.isValid(); ++RI) {
>> +    for (MCSuperRegIterator SI(*RI, MCRI, true); SI.isValid(); ++SI) {
>> +      if (!MO->clobbersPhysReg(*SI))
>> +        return false;
>> +    }
>> +  }
>> +  return true;
>> +}
>> +
>> /// We assume the high bits of a physical super register are not preserved
>> /// unless the instruction has an implicit-use operand reading the
>> /// super-register or a register unit for the upper bits is available.
>> void LiveRegUnits::removeRegsInMask(const MachineOperand &Op,
>>                                     const MCRegisterInfo &MCRI) {
>> -  const uint32_t *Mask = Op.getRegMask();
>> -  unsigned Bit = 0;
>> -  for (unsigned R = 0; R < MCRI.getNumRegs(); ++R) {
>> -    if ((*Mask & (1u << Bit)) == 0)
>> -      removeReg(R, MCRI);
>> -    ++Bit;
>> -    if (Bit >= 32) {
>> -      Bit = 0;
>> -      ++Mask;
>> -    }
>> +  SparseSet<unsigned>::iterator LUI = LiveUnits.begin();
>> +  while (LUI != LiveUnits.end()) {
>> +    if (operClobbersUnit(&Op, *LUI, &MCRI))
>> +      LUI = LiveUnits.erase(LUI);
>> +    else
>> +      ++LUI;
>>   }
>> }
>> 
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits