On 31 May 2013 09:20, Guillermo Polito <[email protected]> wrote:
> Ahh, that's why the code in CoInterpreter I was looking didn't match the C
> code :D. Was looking at the wrong place, hehe.
>
> Igor, why do you say that this method should be reached only with vanilla
> compiled methods? From the comment I understand
>
>       "For interpreter performance and to ease the objectAsMethod
> implementation eagerly
>          evaluate the primtiive, i.e. if the method is cogged and has a
> primitive /do not/ evaluate
>          the machine code primitive, just evaluate primitiveFunctionPointer
> directly."
>

Yes, my bad..
It is hard to see that in code: the logic to handle objects-as-method.
So, yes, please fix executNewMethod and internalExecuteNewMethod with
this additional check.

> And in #addNewMethodToCache: there is as a first statement:
>
> [...]
> (objectMemory isOopCompiledMethod: newMethod)
> ifTrue:
> [primitiveIndex := self primitiveIndexOf: newMethod.
> primitiveFunctionPointer := self functionPointerFor: primitiveIndex inClass:
> class]
> ifFalse:
> [primitiveFunctionPointer := #primitiveInvokeObjectAsMethod].
> [...]
>
> So I'd say that both objects as methods and primitives are kind of handled
> in the same way after?
>
>
>
> On Fri, May 31, 2013 at 2:27 AM, Igor Stasenko <[email protected]> wrote:
>>
>> On 30 May 2013 20:06, Guillermo Polito <[email protected]> wrote:
>> > Ok, I was playing with flushing caches and stuff... Its still a bit
>> > obsure
>> > to me why the example in the workspace works and the code coverage
>> > doesnot.
>> >
>> > However, I managed to hack the VM to make it work :).
>> >
>> > It looks like the VM is treating the objects as methods, as cog methods.
>> > So
>> > I added the following validation in the commonSend:
>> >
>> > methodHeader2 = longAt((GIV(newMethod) + BaseHeaderSize) + (HeaderIndex
>> > <<
>> > ShiftForWord));
>> >
>> > if (isCogMethodReference(methodHeader2) &&
>> > isCompiledMethodHeader(methodHeader2)) {
>> >
>> > /* begin externalizeIPandSP */
>> >
>> > assert((((usqInt)localIP)) != (ceReturnToInterpreterPC()));
>> >
>> > GIV(instructionPointer) = oopForPointer(localIP);
>> >
>> > GIV(stackPointer) = localSP;
>> >
>> > GIV(framePointer) = localFP;
>> >
>> > executeCoggedNewMethodmethodHeader(1, methodHeader2);
>> >
>> > }
>> >
>> >
>> > And it works :).
>> >
>> > I cc Eliot, so maybe he has an idea...
>> >
>> I touched this code.
>>
>> Original:
>>
>> CoInterpreter>>internalExecuteNewMethod
>>         <inline: true>
>>         "For interpreter performance and to ease the objectAsMethod
>> implementation eagerly
>>          evaluate the primtiive, i.e. if the method is cogged and has a
>> primitive /do not/ evaluate
>>          the machine code primitive, just evaluate
>> primitiveFunctionPointer directly."
>>         primitiveFunctionPointer ~= 0 ifTrue:
>>                 [| succeeded |
>>                  self isPrimitiveFunctionPointerAnIndex ifTrue:
>>                         [^self internalQuickPrimitiveResponse].
>>                  "slowPrimitiveResponse may of course context-switch.  If
>> so we must
>> reenter the
>>                   new process appropriately, returning only if we've found
>> an
>> interpreter frame."
>>                  self externalizeIPandSP.
>>                  succeeded := self slowPrimitiveResponse.
>>                  instructionPointer = cogit ceReturnToInterpreterPC
>> ifTrue:
>>                         [instructionPointer := self iframeSavedIP:
>> framePointer].
>>                  self internalizeIPandSP.
>>                  succeeded ifTrue:
>>                         [self return: self popStack toExecutive: true.
>>                          self browserPluginReturnIfNeeded.
>>                         ^nil]].
>>         "if not primitive, or primitive failed, activate the method"
>>         (self methodHasCogMethod: newMethod)
>>                 ifTrue: [self iframeSavedIP: localFP put: localIP
>> asInteger.
>>                                 instructionPointer := cogit
>> ceReturnToInterpreterPC.
>>                                 self externalizeFPandSP.
>>                                 self activateCoggedNewMethod: true.
>>                                 self internalizeIPandSP]
>>                 ifFalse: [self internalActivateNewMethod]
>>
>> Now in subclass (NBCoInterpreter)
>> i made this:
>>
>> internalExecuteNewMethod
>>         <inline: true>
>>         "For interpreter performance and to ease the objectAsMethod
>> implementation eagerly
>>          evaluate the primtiive, i.e. if the method is cogged and has a
>> primitive /do not/ evaluate
>>          the machine code primitive, just evaluate
>> primitiveFunctionPointer directly."
>>
>>         | methodHeader |
>>
>>         methodHeader := self rawHeaderOf: newMethod.
>>
>>         (self isCogMethodReference: methodHeader) ifTrue: [
>>                 self externalizeIPandSP.
>>                 self executeCoggedNewMethod: true methodHeader:
>> methodHeader.
>>                 "we never reach here"
>>                 ].
>>
>>         primitiveFunctionPointer ~= 0 ifTrue:
>>                 [| succeeded |
>>                  self isPrimitiveFunctionPointerAnIndex ifTrue:
>>                         [^self internalQuickPrimitiveResponse].
>>                  "slowPrimitiveResponse may of course context-switch.  If
>> so we must
>> reenter the
>>                   new process appropriately, returning only if we've found
>> an
>> interpreter frame."
>>                  self externalizeIPandSP.
>>                  succeeded := self slowPrimitiveResponse.
>>                  instructionPointer = cogit ceReturnToInterpreterPC
>> ifTrue:
>>                         [instructionPointer := self iframeSavedIP:
>> framePointer].
>>                  self internalizeIPandSP.
>>                  succeeded ifTrue:
>>                         [self return: self popStack toExecutive: true.
>>                          self browserPluginReturnIfNeeded.
>>                         ^nil]].
>>         "if not primitive, or primitive failed, activate the method"
>>         (self methodHasCogMethod: newMethod)
>>                 ifTrue: [self iframeSavedIP: localFP put: localIP
>> asInteger.
>>                                 instructionPointer := cogit
>> ceReturnToInterpreterPC.
>>                                 self externalizeFPandSP.
>>                                 self activateCoggedNewMethod: true.
>>                                 self internalizeIPandSP]
>>                 ifFalse: [self internalActivateNewMethod]
>>
>> ===
>> The intent of change was:
>>  - if method is cog method, execute it (the jited code takes care to
>> call primitive, if it there)
>>
>> The old code logic was:
>>  - run primitive if it there
>>  - and only then, if no prim/or prim failed, try to run a jited code
>> (but entry point is to the first bytecode of method,
>> not to its start)
>>
>> This (old) logic prevents from using primitive 220 properly, because
>> it is a "meta-primitive" - a marker
>> that says that given method should:
>>  - always be jited
>>  - actual primitive is taken by copying machine code from compiled
>> method trailer into CogMethod's primitive section.
>> Therefore executing prim 220 (a C function) ~~ invoking method's
>> primitive (a machine code generated by image).
>>
>> Still i cannot understand why code reaching this point, when a result
>> of lookup is not a compiled method, but arbitrary object, because
>> everything even in original code says: VM enters this method only if
>> it found 100% vanilla guaranteed CompiledMethod, and not something
>> which pretends to be it.
>> As i understand , the logic to handle object-as-method is in method
>> lookup.. but not in internalExecuteNewMethod, because VM can "execute"
>> the only thing it knows, and it is CompiledMethod.
>>
>> Your test says, that Cog does something extra (some magic, which i
>> failed to grasp) with object-as-method,
>> like generating special kind of CogMethod which sends #run:as:in: message
>> to it.
>>
>> I would like to hear Eliot's comments on this.
>>
>>
>>
>> --
>> Best regards,
>> Igor Stasenko.
>>
>



-- 
Best regards,
Igor Stasenko.

Reply via email to