[LLVMdev] LiveIntervals::handlePhysicalRegisterDef, unreachable MBBs

[Alkis Evlogimenos]

On Fri, 2004-07-09 at 05:02, Vladimir Prus wrote:
> I've just spend some time looking at the above function, and while I 
> understood what it does, a comment would have helped a lot.

Thanks for bringing this to my attention!

> Maybe, something like:
> 
> // Determine the end of the live interval for this register
> // For physical register, all internvals are within basic blocks so
> // we look for the instruction in this basic block which last uses it.
> 
> The first loop might have a comment like:
> 
> // If the register is dead at instruction which sets it (i.e. not used later)
> // the live interval ends at the next instruction

Actually it ends at the same instruction. Each instruction has 4 slots.
So instruction 0 spans interval indexes 0-3, instruction 1 4-7 and so
on. In the case above the interval added is:

[defSlot(instr), defSlot(instr)+1)

You may notice that this interval is "empty" in the interval indexes
discrete world (i.e. [13,14)). Indeed it is, and its only purpose is to
naturally express this clobbering of physical registers to the register
allocator. For example given [13,14) for some physical register A and a
live interval [0,27) for some virtual register Z, we cannot allocate A
to Z because of the overlap!

> The second loop might have a comment like:
> 
> // If the register is not dead at the defining instruction, it must be used
> // and killed by some subsequent instruction. Find that instruction now, which
> // always exists

I added comments to the handlePhysicalRegisterDef. Let me know if it
makes more sense now.

> Finally, why I've started all this. I forgot to add machine CFG edge and got 
> misterious crash. I've applied the attached patch, which checks that all 
> basic blocks are reachable from function entry. Any comments on:
> 
> - whether this check is reasonable?
> - what's the right place for the check? LiveVariables.cpp is the simplest I 
> could find, but does not seem right.

I am not sure about this change. I will let others comment on it.

-- 

Alkis