Re: [Lightning] Register allocation semantics

[Paulo César Pereira de Andrade]

2014-09-03 2:05 GMT-03:00 Ian Grant <address@hidden>:
> There is nothing in the manual that either suggests register allocation is
> necessary, nor that it is something that lightning does.

  It is not documented because it is not an exported API :) The prototypes
are not installed either, and are in include/lightning/jit_private.h. And are
used either during construction of the IR or during code generation, due to
backend specific constraints.

> The idea that by restricting to six GP registers and six (or seven?) FP
> registers gives a straightforward, well-defined semantics for all processors
> seems on the face of it a good one.

  The documentation tells about the JIT_R_NUM, JIT_V_NUM and JIT_F_NUM
constants, so that, knowing that one can use JIT_R(index), JIT_V(index), and
JIT_F(index).

  Usually, the larger the JIT_V_NUM the better, because code generation
will have a larger set of callee save registers to hold long living values
without needing to bother with saving/restoring the value on a function
call. But that depends on if the code generation can handle different
values, otherwise, lightning guarantees JIT_R_NUM >= 3, JIT_V_NUM >= 3,
JIT_F_NUM >= 6 and that there is a JIT_FP register as base pointer to
jit_allocai returned addresses, for stack temporaries.

> The recent post suggests that the semantics is in fact not so well-defined.
>
> If the register allocation algorithm has any observable effects above the
> API then those need to be clearly defined because the utility of a library
> depends completely upon the interface being intelligible from the
> documentation, and lightning's is most certainly not. Unfortunately the code
> is such that it is not intelligible through reading that either.

  The observable effect was when correcting a bug, it showed the side
effect of the lightning 2.0 extension of floating point operations with a
constant argument; these per se are not a problem, but branches that
need a temporary, either to computer the target or for a comparison must
use a temporary that should not need to be saved/reloaded. There were
several places where it did not ask for a temporary with the internal
jit_class_nospill bitmask, those did never trigger a problem with the test
cases, but could cause problems on more complex code generation. One
such case was the behaviour after a jump that could not be tracked,
previously it would simply consider all registers live. The change was to
only consider live the registers that are documented as callee save,
because such jumps usually are in some construct that already considers
all register values as dead anyway.

> This is not good. We need to make the core functionality of the tool very
> much more clear than it currently is.
>
> We also need to get it to the point where it doesn't crash the user's
> process. This code is sub-alpha quality, and it should not have been
> released.

  That depends on whether it crashes during jit generation or jit execution :)

> What it needs is a complete ground-up rewrite. The result will be a
> maintainable program which is about 10% of the size of the current release
> and works about ten times better.

  I did on purpose unroll almost every lightning call to make it easier to
read and debug the code, but it is indeed possible to write very tight
instruction
generation code.

> Before we do this we need to clearly specify a guaranteed core semantics,
> and then we can extend this by sub-setting on the supported (fragments of)
> CPU architectures. So we allow the user to specify a class of support and
> then error when they exceed that support. They can then decide whether to
> further restrict the support they require, or work around that missing
> feature at the level of support they need.
>
> None of this is rocket science. All lightning is, is a program that
> constructs strings of bytes. There is no reason whatsoever that such a
> program should ever crash. So we need to make sure that it never ever
> crashes while compiling, whatever crazy sequence of calls the user makes.

  The most common mistake is confusing the "r" and "i" suffixes, and passing
a register for "i" or a constant for "r", for example:

jit_addr(JIT_R0, JIT_R1, 2);  // 2 is a mistake
or
jit_addi(JIT_R0, JIT_R1, JIT_R2); // JIT_R2 is a mistake

  In my fork of lightning 1.2c I changed prototypes to properly use jit_gpr_t
and jit_fpr_t, and then allow compiling as C++ code, because the C++
compiler would warn, or error out on such mistakes. For lightning 2.0
it probably would be required to create a lightning++.h to offer the proper
prototypes.

> Sorry if this sounds harsh criticism, but I consider that I have earned the
> right through the hundred or so hours I've spent trying to find the cause of
> crashes in my code, which invariably turn out to be deficiencies in
> lightning and/or its documentation.  Developer's time is too scarce and
> precious to be wasted like this.

  It is ok :) But please, feel free to report issues you find, or code samples
of the problems, and I will do my best to try to help on correcting the
problems.

> Ian

Thanks,
Paulo