[llvm] r192623 - LiveRegUnits::removeRegsInMask safety.

[Eric Christopher]

> 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.
>

Ick. This seems like complicated badness. I'm also uncertain whether
that says bug in the regmask for the call or an inefficiency somewhere
else.

> 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.

Hrm. I guess that's what I was envisioning this was doing in some
ways. With the register units providing extra clobbers for
instructions that are known to clobber the rest of a register.
Something for the future I guess.

-eric

>
> -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
>