[Vm-dev] StackInterp/Cog>>transferTo: - assertValidExecutionPointe:r:s differences

[Ben Coman]

On 6 May 2018 at 16:56, Ben Coman <btc at openinworld.com> wrote:

>
>
> On 10 June 2016 at 20:17, Eliot Miranda <eliot.miranda at gmail.com> wrote:
>
>>
>> Hi Ben,
>>
>>     this time I'll try and answer your question rather than my brain
>> fart...
>>
>> > On Jun 9, 2016, at 6:46 PM, Ben Coman <btc at openinworld.com> wrote:
>> >
>> >
>> > Just curious about the difference in instructionPointer between these
>> two...
>> >
>> > StackInterpreter>>transferTo:
>> >     self assertValidExecutionPointe: instructionPointer + 1 r:
>> > framePointer s: stackPointer.
>> >
>> >
>> > CoIntepreter>>transferTo:from:
>> >    self assertValidExecutionPointe: instructionPointer r:
>> > framePointer s: stackPointer.
>>
>> In the StackInterpreter, as in the context Interpreter the next bytecode
>> is fetched by pre-incrementing the instructionPointer and then fetching the
>> byte it points to.  Hence it may point to before the first bytecode and
>> hence the + 1 above.  1 is subtracted from a context's pc (2 actually cuz
>> pc is 1-relative and instructionPointer is 0-relative) to derive
>> instructionPointer.
>>
>> In the Cog VM instructionPointer may also be a machine code pc. So rather
>> than add one, which only makes sense for interpreted methods, the
>> CoInterpreter's assertValidExecutionPointer:s: allows for the - 1 if a
>> frame us interpreted.  The StackInterpreter's method predates the
>> CoInterpreter.
>>
>
>
> I was just wandering in the vicinity of this again, and had a new query...
>
> What is the semantic difference in the naming of the arguments (i.e.
> lsp<=> lisp  and  lfp<=>lifp)
> between.... ?
> StackInterpreter >> assertValidExecutionPointe: lip    r: lfp    s: lsp
> imbar: inInterpreter    line: ln
> CoInterpreter     >> assertValidExecutionPointe: lip    r: lifp    s:
> lisp   imbar: inInterpreter    line: ln
>
> because search/replacing makes it easier to diff the two methods...
> https://www.diffchecker.com/CL32EXql
>
> This shows the StackInterpreter method refactored slightly to extract
> "methodField" variable similar to the CoInterpreter
> making it easier to compare the two, i.e. red-line-16 and green-line-18
> are identical,
> but red-line-13 and green-line-14 set "methodField" using different
> messages, which however have identical definitions.
>
> StackInterpreter >> frameMethod: theFP
> <inline: true>
> <var: #theFP type: #'char *'>
> ^stackPages longAt: theFP + FoxMethod
>
> CoInterpreter >> frameMethodField: theFP
> <inline: true>
> <var: #theFP type: #'char *'>
> ^stackPages longAt: theFP + FoxMethod
>
> Would it be reasonable to change red-line-13 to send #frameMethodField:
> instead?  Hopefully my slight rearrangement of the asserts in red okay.
>


========================
I found it a bit convoluted following where CoInterpreter overrides
StackInterpreter >> frameIsBlockActivation:

    StackInterpreter >> frameIsBlockActivation: theFP "<Integer>"
<inline: true>
<var: #theFP type: #'char *'>
^(stackPages byteAt: theFP + FoxFrameFlags + 3) ~= 0

    CoInterpreter >>  frameIsBlockActivation: theFP "<Integer>"
<inline: true>
<var: #theFP type: #'char *'>
^(self isMachineCodeFrame: theFP)
ifTrue: [self mframeIsBlockActivation: theFP]
ifFalse: [self iframeIsBlockActivation: theFP]


But I notice  StackInterpreter >> frameIsBlockActivation:
is identical to...
    CoInterpreter >> iframeIsBlockActivation: theFP "<Integer>"
<inline: true>
<var: #theFP type: #'char *'>
^(stackPages byteAt: theFP + FoxIFrameFlags + 3) ~= 0

while...
StackInterpreter >> iframeIsBlockActivation: theFP "<Integer>"
^self frameIsBlockActivation: theFP

So reversing StackInterpreter's definitions of these two methods thus...
    StackInterpreter >> iframeIsBlockActivation: theFP "<Integer>"
<inline: true>
<var: #theFP type: #'char *'>
^(stackPages byteAt: theFP + FoxFrameFlags + 3) ~= 0

    StackInterpreter >> frameIsBlockActivation: theFP "<Integer>"
^self iframeIsBlockActivation: theFP

makes the existing CoInterpreter override clearer...
    CoInterpreter >>  frameIsBlockActivation: theFP "<Integer>"
<inline: true>
<var: #theFP type: #'char *'>
^(self isMachineCodeFrame: theFP)
ifTrue: [self mframeIsBlockActivation: theFP]
ifFalse: [self iframeIsBlockActivation: theFP]

and also CoInterpreter >> iframeIsBlockActivation becomes redundant.

Can I submit a changeset for these?

cheers -ben
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20180506/a5b26f88/attachment-0001.html>