Class changes do not update dependent words #1751

Open
bjourne opened this Issue Nov 26, 2016 · 4 comments

Projects

None yet

1 participant

@bjourne
Member
bjourne commented Nov 26, 2016 edited
USE: gpu.shaders
[ feedback-format boa ] optimized.
[
    dup [ f ] [ \ t ] if
    [ ] [ maybe{ vertex-format } bad-slot-value ] if {
        feedback-format
        1
        1
        tuple
        91297592
        feedback-format
        -465838109743379406
    } <tuple-boa>
]

And then

USE: gml.viewer ! <- takes a long time
[ feedback-format boa ] optimized.
[
    dup dup [ vertex-format? ] [ drop \ t ] if
    [ ] [ maybe{ vertex-format } bad-slot-value ] if {
        feedback-format
        1
        1
        tuple
        91297592
        feedback-format
        -465838109743379406
    } <tuple-boa>
]

A call to vertex-format? which wasn't there before is added.

@bjourne
Member
bjourne commented Nov 26, 2016

Aha. It's a problem with mixins.

MIXIN: amix
TUPLE: t2 { ah maybe{ amix } } ;
: doit ( a -- b ) t2 boa ;
TUPLE: blah ;
INSTANCE: blah amix 
blah new doit

Bad store to specialized slot
value T{ blah }
class maybe{ amix }

\ doit 1array compile
blah new doit

--- Data stack:
T{ t2 f ~blah~ }

So doit isn't automatically recompiled when the instance is added to the mixin. But it should be.

@bjourne
Member
bjourne commented Nov 30, 2016

It could be that the class algebra with mixins is a little faulty. So this bug is connected to #380.

In doit the call to the predicate word amix? is inlined so the compiler doesn't get the message that the word is changed so it doesn't recompile. It has this dependency check:

T{ depends-on-class-predicate
                { class1
                    intersection{
                        not{ POSTPONE: f }
                        not{ amix }
                    }
                }
                { class2 amix }
            }

The problem with that dependency is that it is always satisfied -> no recompilation. I think what should work is to treat mixins as if they were unions when doing class algebra on them:

UNION: a integer ;
a integer class=
t
MIXIN: b
INSTANCE: integer b
b integer class=
f

I don't see why the result shouldn't be t for mixins too.

@bjourne
Member
bjourne commented Dec 1, 2016

Here is something interesting:

IN: scratchpad [ { fixnum fixnum } declare + ] optimized.
[ fixnum+ ]

Obvious optimization of course. But:

IN: scratchpad MIXIN: mix
IN: scratchpad INSTANCE: fixnum mix
IN: scratchpad [ { mix mix } declare + ] optimized.
[ + ]

So we could optimize it to [ fixnum+ ] here too because the only member of the mixin is fixnum. Then we would have to recompile the callsite when new instances are added to mix.

@bjourne
Member
bjourne commented Dec 6, 2016

Something else interesting:

IN: scratchpad UNION: afix fixnum ;
IN: scratchpad : hi ( x y -- z ) { afix afix } declare + ;
IN: scratchpad \ hi disassemble
00007f413e42c360: 89059a2ce3fe    mov [rip-0x11cd366], eax
00007f413e42c366: 498b06          mov rax, [r14]
00007f413e42c369: 498b5ef8        mov rbx, [r14-0x8]
00007f413e42c36d: 4801c3          add rbx, rax
00007f413e42c370: 0f800e000000    jo 0x7f413e42c384 (hi + 0x24)
00007f413e42c376: 4983ee08        sub r14, 0x8
00007f413e42c37a: 49891e          mov [r14], rbx
00007f413e42c37d: 89057d2ce3fe    mov [rip-0x11cd383], eax
00007f413e42c383: c3              ret
00007f413e42c384: 8905762ce3fe    mov [rip-0x11cd38a], eax
00007f413e42c38a: 488d1d05000000  lea rbx, [rip+0x5]
00007f413e42c391: e9bae531ff      jmp 0x7f413d74a950 (fixnum+overflow)
00007f413e42c396: 0000            add [rax], al
00007f413e42c398: 0000            add [rax], al
00007f413e42c39a: 0000            add [rax], al
00007f413e42c39c: 0000            add [rax], al
00007f413e42c39e: 0000            add [rax], al
IN: scratchpad UNION: afix fixnum float ;
IN: scratchpad \ hi disassemble
00007f413e42c360: 89059a2ce3fe    mov [rip-0x11cd366], eax
00007f413e42c366: 498b06          mov rax, [r14]
00007f413e42c369: 498b5ef8        mov rbx, [r14-0x8]
00007f413e42c36d: 4801c3          add rbx, rax
00007f413e42c370: 0f800e000000    jo 0x7f413e42c384 (hi + 0x24)
00007f413e42c376: 4983ee08        sub r14, 0x8
00007f413e42c37a: 49891e          mov [r14], rbx
00007f413e42c37d: 89057d2ce3fe    mov [rip-0x11cd383], eax
00007f413e42c383: c3              ret
00007f413e42c384: 8905762ce3fe    mov [rip-0x11cd38a], eax
00007f413e42c38a: 488d1d05000000  lea rbx, [rip+0x5]
00007f413e42c391: e9bae531ff      jmp 0x7f413d74a950 (fixnum+overflow)
00007f413e42c396: 0000            add [rax], al
00007f413e42c398: 0000            add [rax], al
00007f413e42c39a: 0000            add [rax], al
00007f413e42c39c: 0000            add [rax], al
00007f413e42c39e: 0000            add [rax], al

Not good, hi wasn't updated and calling 12.3 hi will likely crash.

@bjourne bjourne changed the title from Weird compilation of boa calls to Class changes do not update dependent words Jan 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment