[LLVMdev] Possible VirtRegMap Bug

[Evan Cheng]

On May 27, 2008, at 5:36 PM, David Greene wrote:

> 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


>
>
>                                                 -Dave
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev