Issue 5459 in pharo: Extra love to primitive simulation required

Status: Accepted
Owner: [hidden email]
Labels: Milestone-1.4

New issue 5459 by [hidden email]: Extra love to primitive simulation  
required
http://code.google.com/p/pharo/issues/detail?id=5459

after changes made by Mariano, i can no longer debug NB primitives,
because
a) they need special care when under debugging
b) i used an evil override in simulation to make sure it works

Now, because of override, if you load older version of NativeBoost package,  
it will replace Mariano's code , breaking it.

So, i removed override in last NB config, now i extended ContextPart adding  
a possibility to register own primitive simulators in addition to what is  
present in over-bloated
#doPrimitive:method:receiver:args:
method.

Like that, NativeBoost won't need to override anything, but just register  
own simulator. (and of course any other framework, if needed)


Attachments:
        prim-simulation.1.cs  10.1 KB


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #1 on issue 5459 by [hidden email]: Extra love to primitive  
simulation required
http://code.google.com/p/pharo/issues/detail?id=5459

i also found some suspicious code, when refactoring..
in: doPrimitive: primitiveIndex method: meth receiver: receiver args:  
arguments
there is a check:
                arguments size > 6 ifTrue: [^PrimitiveFailToken].


indicating that you cannot simulate prims which taking more than 6  
arguments..
Now, looking at withoutPrimitiveTryNamedPrimitiveIn: aCompiledMethod for:  
aReceiver withArgs: arguments
there's same check but slightly different:
        arguments size > 8 ifTrue:[^PrimitiveFailToken].

one of them is unnecessary.
Next thing is in tryNamedPrimitiveIn: for:withArgs:
which returns an array:

^{PrimitiveFailToken. ec}

so, it looks like attempt to capture a primitive error code, when it fails.
which means that one primitives when invoked under simulation, and failed
will return PrimitiveFailureToken, while others this array..
but code which verifies that prim failed checks only for PrimitiveFailToken:

        ^value == PrimitiveFailToken
                ifTrue: [PrimitiveFailToken]
                ifFalse: [self push: value]

which means that whole thing should be recondidered:
   1. we should know if simulated primitive failed
   2. we should know if method uses extended pragma for capturing error code
   3. if (2) then we should simulate pushing error code into last temp of  
method before activating it


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Cc: [hidden email]
        Labels: Type-Bug Importance-High

Comment #2 on issue 5459 by [hidden email]: Extra love to primitive  
simulation required
http://code.google.com/p/pharo/issues/detail?id=5459

(No comment was entered for this change.)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Status: FixReviewNeeded

Comment #3 on issue 5459 by [hidden email]: Extra love to primitive  
simulation required
http://code.google.com/p/pharo/issues/detail?id=5459

(No comment was entered for this change.)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #4 on issue 5459 by [hidden email]: Extra love to  
primitive simulation required
http://code.google.com/p/pharo/issues/detail?id=5459

So the issue here is what state the Interpreter VM is in.  Cl;early all the  
simulation methods should use a primitive fail token that includes the  
error code.  In the Qwaq/Teleplace/Newspeak code (where Cog is the only VM)  
I have used

ContextPart class methods for simulation
primitiveFailTokenFor: errorCode

        ^{PrimitiveFailToken. errorCode}

and the code in things like tryNamedPrimitiveIn:for:withArgs:  reads

      arguments size > 8 ifTrue:
                [^self class primitiveFailTokenFor: nil}].

and places where primitive failure is tested (e.g.  
ContextPart>doPrimitive:method:receiver:args:) look like
                 ^(value isArray
                    and: [value size = 2
                    and: [value first == PrimitiveFailToken]])
                        ifTrue: [value]
                        ifFalse: [self push: value]].


But all that depends on tryNamedPrimitiveFor:receiver:args:'s  
implementation in the VM answering error codes as the Cog VM does, using  
negative integers to indicate failures in the invocation of  
tryNamedPrimitiveFor:receiver:args:, and error codes in the invocation of  
the primitive that tryNamedPrimitiveFor:receiver:args invokes.  So we need  
to check that the standard interpreter answers error codes from primitive  
218 (tryNamedPrimitiveFor:receiver:args:) the same way that Cog does.

At least that's why I've held off integrating my Qwaq/Teleplace/Newspeak  
code into Squeak.



_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #5 on issue 5459 by [hidden email]: Extra love to primitive  
simulation required
http://code.google.com/p/pharo/issues/detail?id=5459

yes, that's only one part of story.
the debugger should also simulate properly a primitive failure.
because if debugged method using <primitive:module:>, it should not push an  
error code to stack, because it should be pushed only when method using  
<primitive:module:error:> pragma.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #6 on issue 5459 by [hidden email]: Extra love to  
primitive simulation required
http://code.google.com/p/pharo/issues/detail?id=5459

The error code is only pushed to the stack if the method starts with the  
relevant bytecode.  Look at the attached file activation-methods.st.  They  
check the first bytecode of the method and only store the error code if it  
is a store.

Attachments:
        activation-methods.st  2.7 KB


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #7 on issue 5459 by [hidden email]: Extra love to primitive  
simulation required
http://code.google.com/p/pharo/issues/detail?id=5459

ah, so it is already in place. So, less to worry about.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #8 on issue 5459 by [hidden email]: Extra love to primitive  
simulation required
http://code.google.com/p/pharo/issues/detail?id=5459

btw, there is no implementations nor senders of:
send: selector to: rcvr with: args startClass: startClassOrNil

there's only send: selector to: rcvr with: args super: superFlag
in Pharo... can anyone shed light why we need that?


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #9 on issue 5459 by [hidden email]: Extra love to primitive  
simulation required
http://code.google.com/p/pharo/issues/detail?id=5459

Here's another changeset, which is based on (prim-simulation.1.cs )
Still there are something wrong..

Create a method:

Object>>veryBasicAt: index
        <primitive: 'dooo' module: 'bar' error: code>
        ^ code

now try:
nil veryBasicAt: 999

it answers #'not found' by VM.

Now if you run simulation:


| ctx |
ctx := MethodContext
        sender: nil
        receiver: nil
        method: (Object>>#at: )
        arguments: #(10).
       
        ctx push: nil;
        push: 500;
        doPrimitive: 117  method:  (Object>>#veryBasicAt:) receiver: nil args:  
#(999)


the error code == nil ://


Attachments:
        failtoken-fix.1.cs  11.4 KB


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #10 on issue 5459 by [hidden email]: Extra love to  
primitive simulation required
http://code.google.com/p/pharo/issues/detail?id=5459

The variation between send:to:with:startClass: and send:to:with:startClass:  
is between simulation code that supports Newspeak, where there are  
additional directed sends, and older simulation code that supports only  
Smalltalk.  In any case the point is that which ever of these methods are  
in use they have to answer the continuation context, i.e. either a new  
context to run a new method, or the sending context, if a primitive was  
invoked that succeeded.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #11 on issue 5459 by [hidden email]: Extra love to primitive  
simulation required
http://code.google.com/p/pharo/issues/detail?id=5459

igor you should sit with me (and wait until thursday) or marcus or do  
yourself the integration


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #12 on issue 5459 by [hidden email]: Extra love to primitive  
simulation required
http://code.google.com/p/pharo/issues/detail?id=5459

i will wait you or Marcus. i fear to break system :) because i don't know  
what is a procedure to rollback the update.

i also would like to write some tests for this stuff. because it a bit  
flaky right now :/


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #13 on issue 5459 by [hidden email]: Extra love to primitive  
simulation required
http://code.google.com/p/pharo/issues/detail?id=5459

Please write tests :)



_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #14 on issue 5459 by [hidden email]: Extra love to primitive  
simulation required
http://code.google.com/p/pharo/issues/detail?id=5459

found glitch with simulator.
used { modulename. primname} as key
instead of { primname. modulename }
which prevents a registered simulator for given primitive to be found.

I tested custom simulator implemented with NativeBoost. After this fix, it  
works as expected.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #15 on issue 5459 by [hidden email]: Extra love to primitive  
simulation required
http://code.google.com/p/pharo/issues/detail?id=5459

oops. forgot to attach changeset

Attachments:
        fix-sumilator-glitch.1.cs  819 bytes


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Status: FixToInclude

Comment #16 on issue 5459 by [hidden email]: Extra love to primitive  
simulation required
http://code.google.com/p/pharo/issues/detail?id=5459

(No comment was entered for this change.)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Status: Workneeded

Comment #17 on issue 5459 by [hidden email]: Extra love to primitive  
simulation required
http://code.google.com/p/pharo/issues/detail?id=5459

added tests, hacks to make them green.

To be fixed:
Primitive 218 seems to not correctly return the error code.

We integrated the first 4 changesets (unto this comment) in 14402

Attachments:
        Tests.1.cs  9.2 KB


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Labels: -Milestone-1.4

Comment #18 on issue 5459 by [hidden email]: Extra love to primitive  
simulation required
http://code.google.com/p/pharo/issues/detail?id=5459

I remove the 1.4 tag as it is VM related, not image related


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #19 on issue 5459 by [hidden email]: Extra love to primitive  
simulation required
http://code.google.com/p/pharo/issues/detail?id=5459

So igor what is the status?


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker