Re: amd64Translate.Operand.toAMD64Operand: strange Offset: ...

Thanks for the explanation - very interesting!  I've now got the version 
from master running.  In future I'll work with both master and the last 
release.  (I've just noticed that the helloworld GTK+ app built with 
master causes a seg fault when exiting, so I'll investigate.)

One question - how do I specify a version name when building from 
master?  I tried
   VERSION=<version> make all
but mlton still prints
   MLton ???????? ...
when run.  I must be missing something obvious.

The isNull test is used in my FFI library when converting from a string 
option value to an optionally-null pointer in C.  In the case NONE is 
given explicitly, there will be a test 'isNull Pointer.null', so a 'null 
= null' optimization would remove a little bit of dead code there.

Phil


27/09/15 02:25, Matthew Fluet wrote:
> On Fri, Sep 25, 2015 at 2:38 PM, Phil Clayton <[email protected]> wrote:
>> I was getting the following error when compiling with 20130715:
>>
>> amd64Translate.Operand.toAMD64Operand: strange Offset: base: $0x0 index:
>> MEM<q>{Locals}[(localWord64)+(0x8)]
>>
>> Sometimes the message reported "...+(0x0)".  I have reduced this to an
>> example which is attached.
>>
>> In the process of reducing it, I found an error in my code - an exception
>> would always be raised.  Presumably that is related to this issue because I
>> no longer get this error after fixing my code.
>
> Confirmed with mlton-20130715.  Arguably, it is actually a bug in the
> translation to the Machine IL, since compiling with "-type-check true"
> gives:
>
> [matthew@shadow mlton_compiler_issue-20150925-1]$ mlton
> MLton 20130715 (built Wed Jul 17 03:58:25 EDT 2013 on shadowvm02)
> [matthew@shadow mlton_compiler_issue-20150925-1]$ mlton -type-check
> true mlton.mlb
> invalid operand: XW8 (NULL, RW64(1): Word64, 1, 0): Word8
> invalid statement: RW8(0): Word8  = XW8 (NULL, RW64(1): Word64, 1, 0): Word8
> invalid block: L_181: {kind = Jump,
>        live = (RW64(0): Word64,
>        SP(24): Objptr (opt_8),
>        SW64(32): Word64,
>        SP(48): Objptr (opt_11),
>        SP(40): Objptr (opt_10),
>        SP(16): Objptr (opt_9)),
>        raises = None,
>        returns = Some ()}
>    RW32(0): Word32  = WordU64_extdToWord32 (RW64(0): Word64)
>    RW64(1): Word64  = WordS32_extdToWord64 (RW32(0): Word32)
>    RW8(0): Word8  = XW8 (NULL, RW64(1): Word64, 1, 0): Word8
>    XW8 (SP(24): Objptr (opt_8), RW64(0): Word64, 1, 0): Word8
>     = RW8(0): Word8
>    RW64(2): Word64  = Word64_add (0x1, RW64(0): Word64)
>    RW64(0): Word64  = RW64(2): Word64
>    Goto loop_14
> Machine.typeCheck
>
> The Machine IL program has an array offset operation on a manifestly NULL value.
>
> But, it is already fixed in master:
>
> [matthew@shadow mlton_compiler_issue-20150925-1]$
> ~/devel/mlton/builds/build.gc611cbb/bin/mlton
> MLton gc611cbb (built Mon Aug  3 09:19:31 EDT 2015 on shadow)
> [matthew@shadow mlton_compiler_issue-20150925-1]$
> ~/devel/mlton/builds/build.gc611cbb/bin/mlton mlton.mlb
>
> I believe this was fixed by either:
>    https://github.com/MLton/mlton/commit/444ca04f3630d90e316b0434e52df6c661a84923
> or:
>    https://github.com/MLton/mlton/commit/24b7e0bd003f7435225543b2c798da12109d995a
> Probably the latter, since the commit message notes that the issue
> should only arise in dead code, and you observe that your program
> always raises an exception, so that the array offset operation is dead
> code.
>
> Actually, looking at you program, it also turns out that we are
> missing a reasonable optimization.  MLton is able to constant
> propagate the MLton.Pointer.null value through the program, to the
> point that it is manifestly the base for an array offset operation.
> However, that array offset is guarded by "isNull", but MLton is not
> simplifying a "NULL == NULL" comparison; the RSSA IL program (both
> mlton-20130715 and mlton-master) has:
>
>        x_63: Word32 = CPointer_equal (NULL: CPointer, NULL: CPointer)
>
> If that had been simplified to "true", then the array offset operation
> would have been eliminated as dead code.


------------------------------------------------------------------------------