Possible VirtRegMap Bug

I've been playing around with spillers and found that the SimpleSpiller fails
badly on a particular code.

The problem arises because SimpleSpiller does the test
VRM.isAssignedReg(virtReg) which is implemented as:

00183 bool isAssignedReg(unsigned virtReg) const {
00184 if (getStackSlot(virtReg) == NO_STACK_SLOT &&
00185 getReMatId(virtReg) == NO_STACK_SLOT)
00186 return true;
00187 // Split register can be assigned a physical register as well as a
00188 // stack slot or remat id.
00189 return (Virt2SplitMap[virtReg] && Virt2PhysMap[virtReg] !=
NO_PHYS_REG);
00190 }

VRM::assignVirt2Phys is implemented as:

00147 void assignVirt2Phys(unsigned virtReg, unsigned physReg) {
00148 assert(TargetRegisterInfo::isVirtualRegister(virtReg) &&
00149 TargetRegisterInfo::isPhysicalRegister(physReg));
00150 assert(Virt2PhysMap[virtReg] == NO_PHYS_REG &&
00151 "attempt to assign physical register to already mapped "
00152 "virtual register");
00153 Virt2PhysMap[virtReg] = physReg;
00154 }

Note that this doesn't clear the stack slot or remat mappings.

In the particular code I'm looking at, linear scan spills an interval and
one of the intervals created (%reg1631) gets mark as rematerializable by
LiveIntervals:

00984 if (CreatedNewVReg) {
00985 if (DefIsReMat) {
00986 vrm.setVirtIsReMaterialized(NewVReg, ReMatDefMI/*, CanDelete*/);
00987 if (ReMatIds[VNI->id] == VirtRegMap::MAX_STACK_SLOT) {
00988 // Each valnum may have its own remat id.
00989 ReMatIds[VNI->id] = vrm.assignVirtReMatId(NewVReg);

Linear scan dutifully assigns %reg1631 to AL. Then SimpleSpiller runs and
does this:

00261 unsigned VirtReg = MO.getReg();
00262 unsigned PhysReg = VRM.getPhys(VirtReg);
00263 if (!VRM.isAssignedReg(VirtReg)) {
00264 int StackSlot = VRM.getStackSlot(VirtReg);

Well, according to VRM, %reg1631 is NOT assigned to a register (because it has
a remat id) even though linear scan assigned it one. VRM then returns garbage
as the stack slot and things go downhill from there.

So should VirtRegMap reset the stack slot and remat id maps when it is told to
assign a phys to a virt?

                                                 -Dave

And what happens when SimpleSpiller needs to spill something that
LiveIntervals has given a remat id? SimpleSpiller doesn't do remat and the
stack slot will also be garbage in this case.

                                                   -Dave

I've been playing around with spillers and found that the SimpleSpiller fails
badly on a particular code.

The problem arises because SimpleSpiller does the test
VRM.isAssignedReg(virtReg) which is implemented as:

00183 bool isAssignedReg(unsigned virtReg) const {
00184 if (getStackSlot(virtReg) == NO_STACK_SLOT &&
00185 getReMatId(virtReg) == NO_STACK_SLOT)
00186 return true;
00187 // Split register can be assigned a physical register as well as a
00188 // stack slot or remat id.
00189 return (Virt2SplitMap[virtReg] && Virt2PhysMap[virtReg] !=
NO_PHYS_REG);
00190 }

This is poorly named. All vr's will be assigned a physical register even if they are spilled or remat'd. This really should be isNotSpilledOrReMated. But then the exception is split register. Yeah, I know this is a mess. We are planning a complete rewrite.

VRM::assignVirt2Phys is implemented as:

00147 void assignVirt2Phys(unsigned virtReg, unsigned physReg) {
00148 assert(TargetRegisterInfo::isVirtualRegister(virtReg) &&
00149 TargetRegisterInfo::isPhysicalRegister(physReg));
00150 assert(Virt2PhysMap[virtReg] == NO_PHYS_REG &&
00151 "attempt to assign physical register to already mapped "
00152 "virtual register");
00153 Virt2PhysMap[virtReg] = physReg;
00154 }

Note that this doesn't clear the stack slot or remat mappings.

In the particular code I'm looking at, linear scan spills an interval and
one of the intervals created (%reg1631) gets mark as rematerializable by
LiveIntervals:

00984 if (CreatedNewVReg) {
00985 if (DefIsReMat) {
00986 vrm.setVirtIsReMaterialized(NewVReg, ReMatDefMI/*, CanDelete*/);
00987 if (ReMatIds[VNI->id] == VirtRegMap::MAX_STACK_SLOT) {
00988 // Each valnum may have its own remat id.
00989 ReMatIds[VNI->id] = vrm.assignVirtReMatId(NewVReg);

Linear scan dutifully assigns %reg1631 to AL. Then SimpleSpiller runs and
does this:

00261 unsigned VirtReg = MO.getReg();
00262 unsigned PhysReg = VRM.getPhys(VirtReg);
00263 if (!VRM.isAssignedReg(VirtReg)) {
00264 int StackSlot = VRM.getStackSlot(VirtReg);

Well, according to VRM, %reg1631 is NOT assigned to a register (because it has
a remat id) even though linear scan assigned it one. VRM then returns garbage
as the stack slot and things go downhill from there.

So should VirtRegMap reset the stack slot and remat id maps when it is told to
assign a phys to a virt?

No, every vr is assigned a physical register. It needs to know how what physical register to reload it, for example. Looks like SimpleSpiller has bit-rotted. There really isn't a good reason to fix it. Unless you want to use it, I'll just remove it.

Evan

No, don't remove it! It's a valuable debugging tool. I've got it working
now but am in the middle of hunting this bug I found. Once things have
stabilized here I'll see about submitting the fix.

                                             -Dave

Sounds good. Thanks.

Evan