RE: [Dotgnu-libjit] Libjit bug

[GARCIA DE SORIA LUCENA, JUAN JESUS]

Hi everyone.


I've tracked the issue in my x86-64 GNU/Linux system, and I think I got
a "fix". As I'm not too o acquainted with how libjit works, I'm not sure
that I actually fixed the ultimate cause of the bug.

I've found that the choose_input_order() in jit-reg-alloc.c, when
checking whether the two operands of a commutative does check whether
the second operand already has a register assigned that's the same as
the output register, but doesn't check the case of that assigned
register being a globally assigned register.

I attach the patch and a .tar.bz2 file with the files I used for
testing. I instrumented the libjit code to output many debugging
messages, and to call ndisasm on the code generated for each
instruction. In each test I would compile both AddTest_good.cs ("c = c +
7" version which was already working) and AddTest_bad.cs ("c = 7 + c"
version which failed), and would diff both libjit outputs.

The enclosed good-bad-not-fixed.diff file marks the differences without
the fix.:

        -B39BB126  4183C707          add r15d,byte +0x7
        +B3125126  41BF07000000      mov r15d,0x7
        +B312512C  418BC7            mov eax,r15d
        +B312512F  4403F8            add r15d,eax

The "good" version just adds the immediate byte with value 7 to r15d,
while the bad one ends up overwriting the original r15d value with 7 +
7.

After the changes in my patch, for this instruction the comparison
becomes:

        -4B6A3126  4183C707          add r15d,byte +0x7
        +86D8B126  B807000000        mov eax,0x7
        +86D8B12B  4403F8            add r15d,eax

Now the "bad" output isn't as good as the immediate addition, but at
least the final result is the intended one. This happens because, even
with the IADD opcode marked as being commutative, the generated code
enters through the [reg, reg] case instead of the [reg, imm] one, being
the immediate value the first one, and not the second. I guess that
Klaus' patch would fix this too, but I'm clueless about whether it's the
"right" way or place to do it, not knowing the overall architecture of
libjit.

I've yet to check whether changing "c = 7 + c" into "c = 7 - c" works as
expected, ISUB not being a commutative opcode. I don't know whether the
proper operation of the compilation required select_input_order() to
actually swap the operands, or if some other problem in the register
allocation code lurks beyond it.

Another question I've got after perusing the .ins files for both x86 and
x86-64 architectures is that I found that the floating point opcodes for
x86 for operations like FMUL and DMUL are marked as commutative, whereas
for x86-64 they aren't. Is that an oversight? It could be very well that
it's a different case for x86 and x86-64, and, as I don't know about the
details, I just wanted to make you notice it, just in case.


Best regards,

   Juan Jesus.




> -----Original Message-----
> From: 
> address@hidden 
> [mailto:address@hidden
org] On Behalf Of Aleksey Demakov
> Sent: Monday, June 30, 2008 8:31
> To: Klaus Treichel
> Cc: address@hidden
> Subject: Re: [Dotgnu-libjit] Libjit bug
> 
> Hi Klaus,
> 
> On Mon, Jun 30, 2008 at 1:08 AM, Klaus Treichel 
> <address@hidden> wrote:
> > the attached patch fixes the issue at least for commutative opcodes 
> > with reenabled backward propagation.
> >
> 
> It is more likely that your patch hides the problem rather 
> than fixes it.
> However by itself the patch makes sense as it could improve 
> code generation in some cases.
> 
> Regards,
> Aleksey
> 
> 
> _______________________________________________
> Dotgnu-libjit mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/dotgnu-libjit
>

libjit-input-order-global-reg-fix.diff
Description: libjit-input-order-global-reg-fix.diff

fix-codegen-files.tar.bz2
Description: fix-codegen-files.tar.bz2