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

[Eliot Miranda]

Hi Ben,


> On May 6, 2018, at 1:58 AM, Ben Coman <btc at openinworld.com> wrote:
> 
> 
> 
>> 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?

Thank you, that would be most welcome.  And feel free to include a version of the assert routine that renames lisp & lifp (simple puns) to lsp & lfp or vice verse, as you see fit.

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