[PATCH] PEI scavengeFrameVirtualRegs and RS improvments

[Jakob Stoklund Olesen]

On Mar 23, 2013, at 10:50 PM, Hal Finkel <hfinkel at anl.gov> wrote:

> Some details:
> 
> The existing code supports only one vreg -> physreg mapping at a time, and as it processes the instruction operands in order, it will find the definition of the new virtual register prior to the use of the existing mapping. While this problem can be fixed by scanning the operands in reverse order, that would not help the case where we really have multiple virtual registers that should be simultaneously live. To fix this, I removed the code which maintained the explicit single mapping, and instead call replaceRegWith as soon as the register mapping is established (meaning that RS has scavenged a register). In some sense this changes how the virtual frame scavenging could work: currently a single virtual register, with multiple definitions (non-SSA), could be reused in several places, and scavengeFrameVirtualRegs will happily scavenge a new physical register for each definition. As far as I can tell, only PPC and ARM are using virtual frame scavenging, and neither of those backends use the functionality in this way.

I think it would be a misuse of virtual registers to allow that. The machine code verifier also complains if the live range of a virtual register has multiple connected components. A single virtual register should only be used for a set of operands that must be allocated the same physical register.

I am not sure how clever scavengeFrameVirtualRegs() is, but you should probably make sure that the same scavenged register will be used to rewrite multiple virtual registers as long as their live ranges don't overlap and the scavenged register isn't clobbered in the meantime.

If you use replaceRegWith(), I don't think you'll know which instruction is the last use, and you won't be setting kill flags. Can the scavenger recover after that, or will it think that the rewritten physreg stays alive because there are no kill flags?

Instead of replaceRegWith, you can steal a trick from RAFast: When you rewrite virtual registers top-down, MRI.reg_empty(VirtReg) is going to return true only after you've rewritten the last use of the register. That's the kill, and you can set a kill flag on that last use operand so the scavenger will be able to reuse the register.

> Unfortunately, once the above problem is solved, a code-quality problem surfaces: the RS would never return a register killed by some instruction when asked to scavenge a register at that instruction. This would lead to more use of the spill slots than necessary. To fix this issue I've done two things:
> 
> 1. In scavengeFrameVirtualRegs, RS->forward is called prior to processing the instruction. This makes sure that killed registers are marked as unused. forward() needed some small updates to ignore the virtual registers.
> 
> 2. Normally, scavengeRegister will remove from consideration all registers referenced by the instruction pointed to by the iterator. I've added a boolean parameter to disable this behavior for killed registers (this cannot be made the default behavior because some code inserts code that uses the scavenged register prior to the provided iterator).

This sounds to me like a workaround for an off-by-one error. Shouldn't it be properly defined if the scavenged register is safe to use before or after the current scavenger position?

/jakob