llvm dev - Nov 2014 - [LLVMdev] Stackmaps: caller-save-registers passed as deopt args

This is a follow up on a conversation some of us had at the hacker lab -- I
noticed that sometimes I will get notified that a deopt value lives in a
register that is not callee-save (caller-save I guess, but is there another
term for this that is less similar to callee-save?).  This surprised me
quite a bit since those registers immediately got clobbered by the call
inside the patchpoint, so we were discussing whether this is the
appropriate behavior or not.

[Another point about terminology -- the stackmaps documentation refers to
the non-function-call arguments as "live values", but we were calling
them
"deopt values" in person, and I don't particularly like either
term, since
we also have "live outs" which deal with a separate problem, and for
Pyston
we pass some non-deopt-values in this section.  It might be worth nailing
down a term for this -- personally I call them "stackmap arguments"
and the
initial arguments "function arguments".]

In the cases I'm running into, the value lives in both a register and on
the stack, and the stackmap code seems to preferentially give the register
even if it is a caller-save register.  In our discussion, it seemed like
another possible situation is if the value only lives in a caller-save
register, if it is not used after that patchpoint.  So the questions are:
should we prefer a stack slot to caller-save registers, and should we
prefer them enough to emit a spill if necessary?  I think the answers to
both should be yes.  I think LLVM should emit the spill because otherwise
the client's patchpoint-initialization logic will have to do that;
that's
fine, but it means that one has to reserve nops for spilling in every
patchpoint, whereas LLVM will emit the spill code when necessary.

There was some discussion about what the behavior should be if you use
anyregcc, since in that case it seems reasonable to receive a caller-save
register if you are interested in using it inside the patchpoint.  My
thoughts are that in that case you might be better off including the value
as part of the function arguments instead of the stackmap/deopt arguments.
I think Philip had some more thoughts about how the calling convention of
the patchpoint should apply to the deopt args.

So concretely, I think we should change the lowering so that (other than
anyregcc) the deopt args always get passed on the stack or in callee save
registers, even if it requires adding spills.  I think some of the other
people in the discussion had a better idea about how to potentially
implement that than I did.

kmod
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20141031/ec3898fa/attachment.html>

Hi Kevin,

Thank you for starting this discussion!

I think the distinction is really between whether the live values are
"live on call" or "live on return".  Ideally we should be
able to mark
values to be of either kind (liveoncall vs. liveonreturn) in the call
to the patchpoint intrinsic.  This will make the semantics of
patchpoint slightly odd though -- we'll end up with an SSA instruction
where some inputs are expected to be live *after* the instruction has
been retired.  Are there other SSA instructions that have this sort of
semantics?  Nevertheless, I think splitting the inputs to a patchpoint
into "function arguments", "values live on call" and
"values live on
return" is a good idea.
> There was some discussion about what the behavior should be if you use
> anyregcc, since in that case it seems reasonable to receive a
> caller-save register if you are interested in using it inside the
> patchpoint.  My thoughts are that in that case you might be better off
> including the value as part of the function arguments instead of the
> stackmap/deopt arguments.
I don't see why anything needs to be different for patchpoints running
with anyregcc.  The call arguments always get lowered based on the
calling convention, the "live on call" values get arbitrary stack
slots / registers and the "live on return" values get assigned to CSRs
or stack slots.  If I've read X86RegisterInfo::getCalleeSavedRegs
correctly, I think all registers are callee-saved for anyregcc so
whatever you call using the anyregcc convention will have to respect
that; but that's a different problem.

Thanks!
-- Sanjoy

> On Oct 31, 2014, at 5:28 PM, Sanjoy Das <sanjoy at
playingwithpointers.com> wrote:
> 
> Hi Kevin,
> 
> Thank you for starting this discussion!
Yes, sorry for being unresponsive for a few days. Sanjoy summarized the issues
perfectly.
> I think the distinction is really between whether the live values are
> "live on call" or "live on return".  Ideally we should
be able to mark
> values to be of either kind (liveoncall vs. liveonreturn) in the call
> to the patchpoint intrinsic.  This will make the semantics of
> patchpoint slightly odd though -- we'll end up with an SSA instruction
> where some inputs are expected to be live *after* the instruction has
> been retired.  Are there other SSA instructions that have this sort of
> semantics?
No, it’s a lowering issue. Machine instructions can declare that some registers
are early clobbered so that machine operands can’t use that register. In this
case it’s best just to think about it as a kind of calling convention though.
>  Nevertheless, I think splitting the inputs to a patchpoint
> into "function arguments", "values live on call" and
"values live on
> return" is a good idea.
I basically agree, but think we should have a new intrinsic that is even more
general. We should be able to tag any number of arguments as following some
convention/lowering rules. Those rules should not be baked into the intrinsic.

What Kevin is asking for is very reasonable and arguably a safer default, but we
need the current functionality as well.
As a short term fix, until a new intrinsic is ready, Kevin can add a string
attribute to the patchpoint call, say “live-on-return”.

Some work maybe needed to support this. One possibility is to add a machine
operand flag that is kind of the inverse of EarlyClobber, say LateUse - that’s
hard. An easier option would be to generate a second pseudo instruction
immediately after the patchpoint that simply uses all the live-on-return values.
A cop-out would be to give the call’s register mask operand a different flag so
that is is interpreted as a bunch of early-clobber registers. This last solution
would not allow both live-on-return and live-on-call operands in the same call.
Hopefully there’s something more clever that I haven’t thought of yet.
>> There was some discussion about what the behavior should be if you use
>> anyregcc, since in that case it seems reasonable to receive a
>> caller-save register if you are interested in using it inside the
>> patchpoint.  My thoughts are that in that case you might be better off
>> including the value as part of the function arguments instead of the
>> stackmap/deopt arguments.
> 
> I don't see why anything needs to be different for patchpoints running
> with anyregcc.  The call arguments always get lowered based on the
> calling convention, the "live on call" values get arbitrary stack
> slots / registers and the "live on return" values get assigned to
CSRs
> or stack slots.  If I've read X86RegisterInfo::getCalleeSavedRegs
> correctly, I think all registers are callee-saved for anyregcc so
> whatever you call using the anyregcc convention will have to respect
> that; but that's a different problem.
Yep.

-Andy
> 
> Thanks!
> -- Sanjoy