last "bug"

[Laurent Michel]

Well, it is not really a bug. 

I sometimes emit a jit_movi_p instruction that I later backpatch with the actual value I wish to move into the register. To this end I do something like:

Adr  toPatch =  jit_movi_p(JIT_R0,0);

and later on I patch "toPatch".

This worked fine in my 1.1.2 version of lightning. The macro being defined as:

#define jit_movi_p(d, is) (MOVLir((is), (d)), _jit.x.pc)

where the instruction always assumes a 32-bit wide operand. The macro was rewritten in the head version to:

#define jit_movi_p(d, is)       (jit_movi_l(d, ((long)(is))), _jit.x.pc)

and that does not work. The cast of (is) does not help. The test inside the movi_l basically determines how many bits one needs to store the literal is and issues a suitably chosen opcode to reduce the number of bytes. In my case, since I back-patch, I will end up putting a real 32-bit wide operand during the patching. But since it emitted with the assumption that the operand would fit in 16 bits, it basically overwrite the next two bytes (the next instruction).

I fixed it on my end by doing:

Adr  toPatch =  jit_movi_p(JIT_R0,0x7fffffff);

which forces the emission of a 32-bit wide move. Still, I thought I'd report it as this is a robustness issue. Since the movi_p is already a variant of movi_l that returns the instruction address with the intent to patch it, maybe it would be safer/more robust to always emit an instruction of the maximum operand size.

---

  Laurent

--

  Laurent

_______________________________________________
Lightning mailing list
Lightning@...
http://lists.gnu.org/mailman/listinfo/lightning

[Paolo Bonzini-2]

> Adr  toPatch =  jit_movi_p(JIT_R0,0x7fffffff);
>
> which forces the emission of a 32-bit wide move. Still, I thought I'd
> report it as this is a robustness issue. Since the movi_p is already a
> variant of movi_l that returns the instruction address with the intent
> to patch it, maybe it would be safer/more robust to always emit an
> instruction of the maximum operand size.

Yes, of course.  jit_movi_p is especially meant to be patchable.

Alternatively, make separate 32/64-bit versions that use (respectively)
MOVL and MOVABSQ.  Thanks!

I'll wait for you to commit (remember the ChangeLog and everything) and
then do this.

Paolo


_______________________________________________
Lightning mailing list
Lightning@...
http://lists.gnu.org/mailman/listinfo/lightning

[Laurent Michel]

Ok. Will commit this evening.

On Jun 11, 2008, at 3:33 PM, Paolo Bonzini wrote:

Adr  toPatch =  jit_movi_p(JIT_R0,0x7fffffff);

which forces the emission of a 32-bit wide move. Still, I thought I'd report it as this is a robustness issue. Since the movi_p is already a variant of movi_l that returns the instruction address with the intent to patch it, maybe it would be safer/more robust to always emit an instruction of the maximum operand size.

Yes, of course.  jit_movi_p is especially meant to be patchable.

Alternatively, make separate 32/64-bit versions that use (respectively) MOVL and MOVABSQ.  Thanks!

I'll wait for you to commit (remember the ChangeLog and everything) and then do this.

Paolo

--

  Laurent

_______________________________________________
Lightning mailing list
Lightning@...
http://lists.gnu.org/mailman/listinfo/lightning