Re: [Lightning] SPARC Fixes

[Ludovic Courtès]

Hi Paolo,

Paolo Bonzini <address@hidden> writes:

> Ludovic Courtès wrote:
>>
>>   * Fixed the use of `JIT_RET' (patch-19).  Lightning's `JIT_RET'
>>     actually maps to two registers on SPARC: %i0 is where a procedure's
>>     own return value should be put, and %o0 is where the return value of
>>     a called procedure should be read.  The way I fixed this is simple,
>>     although not very efficient, but I couldn't think of anything
>>     better: any `calli' or `callr' is followed by a `mov %o0, %i0' so
>>     that one can expect %i0 to always contain the callee's return value.
>>
> This is wrong, you have to use jit_retval_i to retrieve %o0 exactly
> for this reason.  It is also wrong because, IIRC, %i0 is a callee-save
> register and you cannot write into it if a function returns void.
> Would you mind preparing a patch-20 to undo the JIT_RET changes?

Ok.  I was under the impression that `jit_retval' was just provided as a
convenience macro that is a shortcut for `jit_movi_p (JIT_RET, xxx)'.

Patches 20 to 22 undo the change, fix the test case, and clarify the
documentation about `JIT_RET' and `jit_retval'.

> Another possibility (simpler for now to implement) is that if
> JIT_MAX_STACK_AUTOMATIC_AREA is defined, clients should be careful not
> to push more than that amount of words.

You mean we should just clearly document this macro?

> Or we could just deprecate
> pushr/popr and define a better way to provide stack space for spills.

For 1.2, we have to keep them anyway.  Even beyond, is may be a nice
thing to have because it probably looks familiar to many people.

>>     Under `_ASM_SAFETY', `pushr' "overflows" and `popr' "underflows" are
>>     detected.  I think we should document `_ASM_SAFETY' and perhaps
>>     define it by default.
>>
> That's --enable-assertions.  :-)

Yes, but my program uses the version of lightning that's available on
the host machine (i.e., it not bundle lightning), so I don't have access
to this Autoconf machinery.

>>     Instead of having a hard limit on the number of registers that may
>>     be pushed, we could also patch the `save' instruction based on the
>>     actual number of `pushr' encountered in the function's code, but I'm
>>     not sure it's worth it.
>>
> That would be the way to provide stack space for spills -- a lightning
> instruction patching the `save' on the SPARC, and on other
> architectures compiling to subr JIT_SP, JIT_SP, N.

What does "spills" mean here?

I haven't looked at instruction encoding so I have no idea whether
patching `save' would be an easy task (I guess it shouldn't be too
hard).  Would you be in favor of patching `save' rather than having this
fixed limit?

Thanks,
Ludovic.