[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Lightning] Bug in testing instructions.

From: Laurent Michel
Subject: [Lightning] Bug in testing instructions.
Date: Tue, 10 Jun 2008 19:40:27 -0400

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/ 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? 



Attachment: smime.p7s
Description: S/MIME cryptographic signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]