On 31 May 2013 09:20, Guillermo Polito <guillermopolito@gmail.com> 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 <siguctua@gmail.com> wrote:
>>
>> On 30 May 2013 20:06, Guillermo Polito <guillermopolito@gmail.com> 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.