[LLVMdev] Patchpoints used for inline caches and pointless reloads

[Philip Reames]

On 02/25/2015 06:45 AM, Frej Drejhammar wrote:
> Philip Reames <listmail at philipreames.com> writes:
>
>> Just to make sure we're on the same page, let me try to rephrase your
>> problem:
>> "After the initial call is patched out of existence, the register
>> spilling which was reasonable for the initial call becomes a
>> performance liability for the patched in assembly."
>>
>> Is that a correct restatement?
> That is correct.
>
> *trim for length*
>> What I think you really want is a parameter attribute that we don't
>> have.  You want to be able to tag a particular parameter as being
>> "anylocation" (analogous to the anyreg of anyregcc).  In concept, a
>> "anystack" notion is similar to what we do for gc.statepoints (and, I
>> think, excess arguments on patchpoints?).  That's still not quite as
>> strong as what you want though...
> Since I sent the original email I have been working on a pass inspired
> by LiveDebugVariables. Before register allocation it notes the virtual
> register for each non-lowered patchpoint operand and then replaces the
> operand with an immediate. During register allocation the pass tracks
> the original virtual register and when register allocation is complete,
> the operands are replaced with a physical register or a stack slot. That
> way the arguments to the patchpoint does not influence register
> allocation at all, i.e. when the patchpoint call is patched out it will
> be as if it never existed.
Oh, cool.  Do you have code you could share?  I'd really like to see how 
this approach works out.  This is *exactly* the behavior we'd like to 
have by default for gc.statepoint.  I hadn't thought of this approach 
before.
> Instead of treating all non-lowered operands like this, I can simply
> limit it to parameters marked with an "anylocation" parameter
> attribute. That is a good suggestion for extending the patchpoint
> behavior.
Just to be clear, I'm not set on having the parameter attribute.  If you 
can make everything work and get Andy to sign off on the behavior change 
for patchpoint, that's also fine by me.

>
>> If you do decide to pursue the "anylocation" parameter attribute,
>> definitely keep me informed. This is analogous to something we want
>> for gc.statepoints as well.
> I'll be happy to send you my current patch off-list if you are
> interested. I'll definitely add you as a reviewer when I try to push it
> upstream. Can you tell me more about what it is you want for
> gc.statepoints?
Please do.

Essentially, we'd like exactly what you're describing.  Register 
allocation should ignore uses in the gc.statepoint.  The gc.statepoint 
needs to be updated to know where a particular virtual register got 
placed, but that's it.  We're perfectly happy with values being placed 
in registers or stack slots.

(Well, mostly.  Putting gc references in registers needs to be optional 
since many runtimes won't support it.)
>
>> p.s. Without knowing what DB actually is, it really looks like you
>> might be able to accomplish the same thing using the "ID" field of the
>> patchpoint.  Have you looked into that?
> Sure, an extra indirection solves all problems in CS :) But it is not
> that simple if, for example, values the patch mechanism needs are passed
> as arguments to function containing the patchpoint.
Figured there's be a complication somewhere.  Personally, I'm glad the 
simple solution doesn't work for you; it sounds like you're solving a 
problem I'd really like to see solved.  :)
>
> Cheers,
>
> --Frej