[llvm-commits] [llvm] r153903 - in /llvm/trunk/lib/CodeGen: InlineSpiller.cpp LiveRangeEdit.cpp LiveRangeEdit.h RegAllocBasic.cpp RegAllocGreedy.cpp RegAllocPBQP.cpp Spiller.cpp SplitKit.cpp

[Peter Cooper]

Thanks for the explanation Jakob.  Fixed in r153914.

Pete
On Apr 2, 2012, at 4:13 PM, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote:

> 
> On Apr 2, 2012, at 4:00 PM, Peter Cooper <peter_cooper at apple.com> wrote:
> 
>>>> @@ -282,12 +264,14 @@
>>>>  // Shrink just one live interval. Then delete new dead defs.
>>>>  LiveInterval *LI = ToShrink.back();
>>>>  ToShrink.pop_back();
>>>> -    if (foldAsLoad(LI, Dead, MRI, LIS, TII))
>>>> +    if (foldAsLoad(LI, Dead))
>>>>    continue;
>>>>  if (delegate_)
>>>>    delegate_->LRE_WillShrinkVirtReg(LI->reg);
>>>>  if (!LIS.shrinkToUses(LI, &Dead))
>>>>    continue;
>>>> +    if (!VRM)
>>>> +      continue;
>>> 
>>> This looks suspicious. The code below does more than just updating VRM.
>> Yeah, it is a little suspicious.  The check before this one avoids running that code if the range was being spilled.  I assumed that if its ok to avoid this code in that case then it should be ok to avoid it here too.  However, I'm happy to be proved wrong and learn more about the register allocator.
> 
> The code below will detect if the live range separates into multiple connected components, and create new virtual registers for the new components. That should always happen whether a VRM needs to be updated or not.
> 
> The machine code verifier will complain about virtual registers with multiple connected components. The special case for the spiller only works because the spiller is about to eliminate those virtual registers anyway.
> 
> You shouldn't use the VRM pointer to change the behavior of LRE, just change whether it updates VRM itself.
> 
> /jakob
>