[Lightning] Bug in testing instructions.

[Laurent Michel]

I have in my code sequence like this:

void LocInstrIGEQI::compile(ColJitServicesApi* jit,CotJit* table)

{

   _start = jit->forward();

   jit->ldxi_i(CJIT_R0,CJIT_V0,-sizeof(ColSlotI));       // R0 <- V0{SP}[-1]

   jit->ldxi_i(CJIT_R1,CJIT_V0,-2*sizeof(ColSlotI));     // R1 <- V0{SP}[-2]

   jit->ger_i(CJIT_R0,CJIT_R1,CJIT_R0);                  // R0 <- R1 >= R0

   jit->stxi_i(-2*sizeof(ColSlotI),CJIT_V0,CJIT_R0);     // V0{SP}[-2] <- R0

   jit->subi_i(CJIT_V0,CJIT_V0,sizeof(ColSlotI));        // VO{SP} <- VO{SP} - 1      

}

The important bit here is the ger_i instruction that is supposed to perform a test between R1 and R0 and set the result in R0 (for a subsequent branch).

The sequence emitted for this specific block was:

L020910951]0x20208190[0x2031c1bb][ book/queens.co: 40] igeq

2031c1bb movl 0xfffffff0(%ebx),%eax

2031c1be movl 0xffffffe0(%ebx),%ecx

2031c1c1 xorl %eax,%eax

2031c1c3 cmpl %eax,%ecx

2031c1c5 setge %al

2031c1c8 movl %eax,0xffffffe0(%ebx)

2031c1cb addl $0xfffffff0,%ebx

If you focus on the ger related sequence you see:

2031c1c1 xorl %eax,%eax

2031c1c3 cmpl %eax,%ecx

2031c1c5 setge %al

Which is unfortunately incorrect because:

1) it zeroes eax (so we loose one of the operand

2) it does the test after zeroing.

If you switch the two:

cmpl %eax,%ecx

xorl %eax,%eax

setge %al

it is still incorrect because (as far as I can tell) xorl resets the condition code.

The sequence with the previous version of lightning gave me:

2031c3a4 cmpl %eax,%ecx

2031c3a6 movl $0x0,%eax

2031c3ab setge %al

which resets eax _after_ the cmpl and the mov instruction leaves the condition code alone. 

So, I dug in the code and changed the jit_replace8 macro in core-i386.h (~ line 72) to

#define jit_replace8(d, cmp, op) \

(jit_check8(d) \

 ? ((cmp), \

             MOVLir(0, d),             \

             op(_rN(d) | _AL))         \

 : (jit_pushr_i(_EAX), (cmp), \

             MOVLir(0, d),                                      \

             op(_AL), MOVLrr(_EAX, (d)), jit_popr_i(_EAX)))

which revers to the previous mov based initialization. 

If there is a better fix, I'm all ears. If not, would you consider integrating? 

Thanks!

--

  Laurent

smime.p7s
Description: S/MIME cryptographic signature