[llvm] r178845 - Fix bug in PEI's virtual-register scavenging

[Hal Finkel]

----- Original Message -----
> From: "Krzysztof Parzyszek" <kparzysz at codeaurora.org>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: llvm-commits at cs.uiuc.edu
> Sent: Friday, April 5, 2013 2:35:06 PM
> Subject: Re: [llvm] r178845 - Fix bug in PEI's virtual-register scavenging
> 
> On 4/5/2013 2:09 PM, Hal Finkel wrote:
> >
> > I think that it is better to keep the RS as consistent as possible.
> > If we ever remove the FIXME in RS and it starts verifying defs as
> > well, this way it won't blow up.
> 
> I don't think there is anything inconsistent with my approach.
>  Here's
> my logic:
> 
> By the time when we do the scavenging, the RS would have scanned the
> instruction (so the physical register operands would have been
> recorded).  The def (or a virtual register) would be skipped at that
> time.  In the generated spill code, the use of the scavenged register
> would be marked as a kill, and so the newly substituted def would be
> consistent with the liveness information.  The PEI code would then
> mark
> the register as used (as it does now[1]).  These two steps would
> evaluate to a no-op from the RS's perspective, so it should be safe
> to
> simply reset the RS to the next instruction.
> 
> [1] The "RS->setUsed(ScratchReg)" should be guarded by "if
> (!MO.isDead())".

I think this would work, but does scare me slightly (which is why I did not do it). Looking at the RS forward() function, it looks like I might need to do something more complicated for predicated instructions (and perhaps instructions with register masks). My thought was that is was better not to make PEI guess the behavior of RS any more than absolutely necessary, and to keep everything as consistent as possible. Nevertheless, I'm certainly open to fixing this in an alternative way. The benefit of doing it this way is that I just had to refactor, not duplicate, the instruction processing logic inside RS. In short, I felt that this way is "safer."

Thanks again,
Hal

> 
> -Krzysztof
> 
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by The Linux Foundation
>