[Lightning] Re: Updates for x86_64

[Paulo Cesar Pereira de Andrade]

On 24-08-2010 07:38, Paolo Bonzini wrote:

[sorry for the delay, moving mandriva servers, reconfigured my mail,
 joined @lightning with another email also :-)]

> On 08/24/2010 01:12 AM, Paulo César Pereira de Andrade wrote:
> > Patch1 is a clear typo and should be trivial.
>
> Ok.
>
> > Patch2 changes JIT_REXTMP to %r11, so that, when calling a function
> > with 6 integer arguments, it will not clobber the 6th argument in the
> > last moment, due to putting the function pointer on it.
> >
> > /* Used to implement ldc, stc, ... */
> > #define JIT_CAN_16 0
> > -#define JIT_REXTMP _R9D
> > +#define JIT_REXTMP _R11D
> >
> > #define JIT_R_NUM 3
> > #define JIT_R(i) ((i) == 0 ? _EAX : _R9D + (i))
>
> Doesn't this mean JIT_R2 overlaps JIT_REXTMP? Please redo the patch
> using %r12, and at the same time moving V1/V2 to r13/r14.

  Ok. This looks like the most plausible solution :-) Actually, I think
JIT_REXTMP should be more like a variable, but things to do at a later
time.

> > Patch3 defines symbolic names for extra SSE2 %xmmN registers, and
> > sets JIT_FPTMP to the topmost one.
>
> Ok.

  Following logic of JIT_REXTMP, I will reduce JIT_FPR_NUM by one then,
to overlapping.

> > Patch4 increases JIT_V_NUM to 5, and make %r14 and %r15 available,
> > at the cost of needing to also save them. They are callee save registers
> > in the abi.
>
> I'd rather skip this one, 3 callee-save register are often enough. %r14
> is going to be used anyway for V2 after you redo patch 2.

  It still needs to be saved, but by not using %r15, can keep the stack
aligned at 16 bytes without pushing %rbx a second time just to align.
Either way this can be reviewed later.

> > Patch5 adds safety check on number of integer or float arguments
> > being passed to a function, and also increases the number of float
> > register arguments to 8. Also, it defines JIT_RA_NUM and JIT_FA_NUM
> > that matches the number of integer and float register arguments.
> >
> > Patch6 also changes the mapping of JIT_R(num) and JIT_V(num), as well
> > as JIT_REXTMP to use the 64 bits defines, so that it will not trigger
> > an JITFAIL on jit_getarg_l and jit_getarg_ul for example, due to _rC
> > telling it is a 32 bits registers. This is only an issue when
> > _ASM_SAFETY is defined.
>
> Both look fine.

  I believe it may be required some changes in macros like _r4, _r8,
etc, not to fail if using _AL, instead of _EAX, or _RAX instead of
_EAX, and the like, but to check if the "base" register is valid,
example, instead of having:

#define _r4(R)		( (_rC(R) == 0x40)			? _rN(R) : JITFAIL("32-bit register 
required"))

it should be more like:

#define _r4(R)  assert_has_32_bit_register(_rN(R))

But in some cases, it may be too late, as it may be checking not
to ensure it passed the proper name, but to check that specific
register is suitable for a given operation.

> Paolo

  I will remake the patches up to patch6 and resend them shortly.

Paulo