VM Maker: VMMaker.oscog-eem.2222.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

VM Maker: VMMaker.oscog-eem.2222.mcz

commits-2
 
Eliot Miranda uploaded a new version of VMMaker to project VM Maker:
http://source.squeak.org/VMMaker/VMMaker.oscog-eem.2222.mcz

==================== Summary ====================

Name: VMMaker.oscog-eem.2222
Author: eem
Time: 26 May 2017, 3:24:23.911304 pm
UUID: 72a78cd4-0916-4b02-afea-75e4ee736bac
Ancestors: VMMaker.oscog-nice.2221

StackInterpreter:
Add time primitives to answer the UTC and local microsecond clocks as updated by the heartbeat (as the Cog VM was originally before David changed the primtiives to access the time now).  This access is much faster (of the order of 20 nsecs per invocartion vs 450 nsecs per invocatuon on a 2.3 GHz Mac Mini).  Terf demands very fast clock times but is not concerned with fine granularity.  The primitive numbers are 245 for UTC and 246 for local.

primitiveMakePoint should fail if its /argument/ is not a SmallInteger or Float.  Since the primitive is on Number we know the receiver is valid, and there is no point chekcing it.  Checking the argument allows us to e.g. make 3D point using a cascade of @'s.

Fix warnings for addIdleUsecs: on WIN32/64 (must not be marked exporrt to avoid declspec being added).

reverseDisplayFrom:to: slightly improved if it does the addition at start of loop, not on every iteration.

Cogit:
primitiveCollectCogCodeConstituents should verify its argument (if it has one) and fail if not given a boolean.  If it is given an argument it must remove it from the stack.

Correct comment in sendTrace:.

=============== Diff against VMMaker.oscog-nice.2221 ===============

Item was changed:
  ----- Method: CoInterpreterPrimitives>>primitiveCollectCogCodeConstituents (in category 'process primitives') -----
  primitiveCollectCogCodeConstituents
+ "Answer the contents of the code zone as an array of pair-wise element, address in ascending
+ address order. Answer a string for a runtime routine or abstract label (beginning, end, etc),
+ a CompiledMethod for a CMMethod, or a selector (presumably a Symbol) for a PIC.
+ If there is an argument and it is true, then collect inner information about the CogMethod."
- "Answer the contents of the code zone as an array of pair-wise element, address in ascending address order.
- Answer a string for a runtime routine or abstract label (beginning, end, etc), a CompiledMethod for a CMMethod,
- or a selector (presumably a Symbol) for a PIC.
- If the last argument (or the receiver if no arguments) is true, then collect inner information about the CogMethod."
  | constituents withDetails |
+ argumentCount = 0
+ ifTrue: [withDetails := false]
+ ifFalse:
+ [withDetails := self stackTop.
+ (withDetails = objectMemory trueObject
+  or: [withDetails = objectMemory falseObject]) ifFalse:
+ [^self primitiveFailFor: PrimErrBadArgument].
+  withDetails := withDetails = objectMemory trueObject].
+ constituents := cogit cogCodeConstituents: withDetails = objectMemory trueObject.
- withDetails := self stackTop.
- constituents := cogit cogCodeConstituents: (withDetails = objectMemory trueObject).
  constituents ifNil:
  [^self primitiveFailFor: PrimErrNoMemory].
+ self pop: argumentCount + 1 thenPush: constituents!
- self pop: 1 thenPush: constituents!

Item was changed:
  ----- Method: Cogit>>sendTrace: (in category 'debugging') -----
  sendTrace: aBooleanOrInteger
  <doNotGenerate>
  "traceFlags is a set of flags.
  1 => print trace (if something below is selected)
  2 => trace sends
  4 => trace block activations
  8 => trace interpreter primitives
  16 => trace events (context switches, GCs, etc)
  32 => trace stack overflow
  64 => send breakpoint on implicit receiver (Newspeak VM only)
  128 => check stack depth on send (simulation only)
+ 256 => trace linked sends "
- 256 => count sends (simulation only)"
  traceFlags := aBooleanOrInteger isInteger
  ifTrue: [aBooleanOrInteger]
  ifFalse: [aBooleanOrInteger ifTrue: [6] ifFalse: [0]]!

Item was added:
+ ----- Method: InterpreterPrimitives>>primitiveCoarseLocalMicrosecondClock (in category 'system control primitives') -----
+ primitiveCoarseLocalMicrosecondClock
+ "Return the value of the microsecond clock in the local timezone, as updated by the heartbeat, as an integer.
+ This is the number of microseconds since the Smalltalk epoch, 1901/1/1 12:00am.
+ The microsecond clock is at least 60 bits wide which means it'll get to around August
+ 38435 before it wraps around.  Be sure to put it on your calendar.  The coarse clock is
+ updated by the heartbeat thread and as such is much cheaper than
+ primitiveUTCMicrosecondClock, which always entails a system call."
+
+ self pop: 1 thenPush: (self positive64BitIntegerFor: self ioLocalMicroseconds)!

Item was added:
+ ----- Method: InterpreterPrimitives>>primitiveCoarseUTCMicrosecondClock (in category 'system control primitives') -----
+ primitiveCoarseUTCMicrosecondClock
+ "Return the value of the microsecond clock as updated by the heartbeat as an integer.
+ This is the number of microseconds since the Smalltalk epoch, 1901/1/1 12:00am.
+ The microsecond clock is at least 60 bits wide which means it'll get to around August
+ 38435 before it wraps around.  Be sure to put it on your calendar.  The coarse clock is
+ updated by the heartbeat thread and as such is much cheaper than
+ primitiveUTCMicrosecondClock, which always entails a system call."
+
+ self pop: 1 thenPush: (self positive64BitIntegerFor: self ioUTCMicroseconds)!

Item was changed:
  ----- Method: InterpreterPrimitives>>primitiveLocalMicrosecondClock (in category 'system control primitives') -----
  primitiveLocalMicrosecondClock
+ "Return the value of the microsecond clock in the local timezone as an integer.
+ This is the number of microseconds since the Smalltalk epoch, 1901/1/1 12:00am.
+ The microsecond clock is at least 60 bits wide which means it'll get to around August
+ 38435 before it wraps around.  Be sure to put it on your calendar.  This primitive
+ accesses the time as answered by the OS."
- "Return the value of the microsecond clock as an integer.  The microsecond clock is at
- least 60 bits wide which means it'll get to around August 38435 before it wraps around."
 
  self pop: 1 thenPush: (self positive64BitIntegerFor: self ioLocalMicrosecondsNow)!

Item was changed:
  ----- Method: InterpreterPrimitives>>primitiveMakePoint (in category 'arithmetic integer primitives') -----
  primitiveMakePoint
  <inline: false>
+ | rcvr arg pt |
- | rcvr pt |
  rcvr := self stackValue: 1.
+ arg := self stackTop.
+ ((objectMemory isIntegerObject: arg) or: [objectMemory isFloatObject: arg]) ifFalse:
+ [^self primitiveFailFor: PrimErrBadArgument].
- ((objectMemory isIntegerObject: rcvr) or: [objectMemory isFloatObject: rcvr]) ifFalse:
- [^self primitiveFail].
  pt := objectMemory eeInstantiateSmallClass: (objectMemory splObj: ClassPoint) numSlots: YIndex + 1.
+ objectMemory "No need to check since new object is always new."
- objectMemory
  storePointerUnchecked: XIndex ofObject: pt withValue: rcvr;
+ storePointerUnchecked: YIndex ofObject: pt withValue: arg.
- storePointerUnchecked: YIndex ofObject: pt withValue: self stackTop.
  self pop: 2 thenPush: pt!

Item was changed:
  ----- Method: InterpreterPrimitives>>primitiveUTCMicrosecondClock (in category 'system control primitives') -----
  primitiveUTCMicrosecondClock
+ "Return the value of the microsecond clock in UTC as an integer.
+ This is the number of microseconds since the Smalltalk epoch, 1901/1/1 12:00am.
+ The microsecond clock is at least 60 bits wide which means it'll get to around August
+ 38435 before it wraps around.  Be sure to put it on your calendar.  This primitive
+ accesses the time as answered by the OS."
- "Return the value of the microsecond clock as an integer.  The microsecond clock is at
- least 60 bits wide which means it'll get to around August 38435 before it wraps around."
 
  self pop: 1 thenPush: (self positive64BitIntegerFor: self ioUTCMicrosecondsNow)!

Item was changed:
  ----- Method: StackInterpreter class>>initializePrimitiveTable (in category 'initialization') -----
(excessive size, no diff calculated)

Item was changed:
  ----- Method: StackInterpreter>>addIdleUsecs: (in category 'primitive support') -----
  addIdleUsecs: idleUsecs
  "The various poll/select calls in the VM should attempt to tally the ammount
  of time spent at idle here, so as to render the uptime value meaningful."
+ <api>
- <export: true>
  statIdleUsecs := statIdleUsecs + idleUsecs!

Item was changed:
  ----- Method: StackInterpreter>>reverseDisplayFrom:to: (in category 'I/O primitive support') -----
  reverseDisplayFrom: startIndex to: endIndex
  "Reverse the given range of Display pixels, rounded to whole word boundary.
  Used to give feedback during VM activities such as garbage collection when debugging.
  It is assumed that the given word range falls entirely within the first line of the Display."
 
  | wordStartIndex wordEndIndex primFailCodeValue |
+ (displayBits = 0 or: [(objectMemory isImmediate: displayBits asInteger) or: [displayDepth <= 0]]) ifTrue: [^nil].
- (displayBits = 0 or: [(objectMemory isImmediate: displayBits asInteger) or: [displayDepth = 0]]) ifTrue: [^nil].
  wordStartIndex := (startIndex max: 0) * displayDepth // 32.
  wordEndIndex := (endIndex min: displayWidth) * displayDepth // 32.
+ displayBits asInteger + (wordStartIndex * 4) to: displayBits asInteger + (wordEndIndex * 4) by: 4 do:
+ [:ptr | | reversed |
+ reversed := (objectMemory long32At: ptr) bitXor: 16rFFFFFFFF.
+ objectMemory long32At: ptr put: reversed].
- wordStartIndex * 4 to: wordEndIndex * 4 do:
- [:byteOffset | | reversed |
- reversed := (objectMemory long32At: displayBits asInteger + byteOffset) bitXor: 16rFFFFFFFF.
- objectMemory long32At: displayBits asInteger + byteOffset put: reversed].
  primFailCodeValue := primFailCode.
  self initPrimCall.
  self updateDisplayLeft: 0 Top: 0 Right: displayWidth Bottom: 1.
  self ioForceDisplayUpdate.
  primFailCode := primFailCodeValue!

Reply | Threaded
Open this post in threaded view
|

Re: VM Maker: VMMaker.oscog-eem.2222.mcz

Juan Vuletich-3
 
Hi Eliot, Folks,

I think this commit introduces a bug:

On 5/26/2017 10:25 PM, [hidden email] wrote:
> primitiveMakePoint should fail if its /argument/ is not a SmallInteger or Float.  Since the primitive is on Number we know the receiver is valid, and there is no point chekcing it.  Checking the argument allows us to e.g. make 3D point using a cascade of @'s.
>

The reason why this is wrong is that #@ is a special selector and is
just not usually sent, being replaced by bytecode 187 /*
bytecodePrimMakePoint */. This means that the primitive will be called
with receivers of any kind, not just Number. This breaks the 3D point
use case:

Previously, doing (1 @ 2) @ 3 would create the Point(1,2) and send #@ 3.
The primitive would fail because of check of receiver, and a normal send
would be done, creating an appropriate 3D point.

With this change, the receiver is not validated, but the argument is.
The primitive doesn't fail (arg is number) and the result is a Point
with another point as x: Point(Point(1,2),3). In the explorer it shows:
1@2@3
     x: 1@2
         x: 1
         y: 2
     y: 3

I think that validating the argument is good, but we also need to
validate receiver.

Thanks,

--
Juan Vuletich
www.cuis-smalltalk.org
https://github.com/Cuis-Smalltalk/Cuis-Smalltalk-Dev
@JuanVuletich


Reply | Threaded
Open this post in threaded view
|

Re: VM Maker: VMMaker.oscog-eem.2222.mcz

Eliot Miranda-2
 
Hi Juan,


> On Jun 5, 2017, at 9:26 AM, Juan Vuletich <[hidden email]> wrote:
>
> Hi Eliot, Folks,
>
> I think this commit introduces a bug:
>
>> On 5/26/2017 10:25 PM, [hidden email] wrote:
>> primitiveMakePoint should fail if its /argument/ is not a SmallInteger or Float.  Since the primitive is on Number we know the receiver is valid, and there is no point chekcing it.  Checking the argument allows us to e.g. make 3D point using a cascade of @'s.
>>
>
> The reason why this is wrong is that #@ is a special selector and is just not usually sent, being replaced by bytecode 187 /* bytecodePrimMakePoint */. This means that the primitive will be called with receivers of any kind, not just Number. This breaks the 3D point use case:
>
> Previously, doing (1 @ 2) @ 3 would create the Point(1,2) and send #@ 3. The primitive would fail because of check of receiver, and a normal send would be done, creating an appropriate 3D point.
>
> With this change, the receiver is not validated, but the argument is. The primitive doesn't fail (arg is number) and the result is a Point with another point as x: Point(Point(1,2),3). In the explorer it shows:
> 1@2@3
>    x: 1@2
>        x: 1
>        y: 2
>    y: 3
>
> I think that validating the argument is good, but we also need to validate receiver.

Damn, you're right.  I had forgotten about the special selector case.  The non/special selector form (a send in jotted code or a perform:) validated the receiver implicitly because the problem motive method is on Number (and Points are not numbers).  But when used as a special selector in interpreted code the receiver does still need validating.  I will fix this today.  Thanks for spotting my my mistake!

>
> Thanks,
>
> --
> Juan Vuletich
> www.cuis-smalltalk.org
> https://github.com/Cuis-Smalltalk/Cuis-Smalltalk-Dev
> @JuanVuletich
>
>
Reply | Threaded
Open this post in threaded view
|

Re: VM Maker: VMMaker.oscog-eem.2222.mcz

Denis Kudriashov
 
Hi

2017-06-05 18:46 GMT+02:00 Eliot Miranda <[hidden email]>:
Damn, you're right.  I had forgotten about the special selector case.  The non/special selector form (a send in jotted code or a perform:) validated the receiver implicitly because the problem motive method is on Number (and Points are not numbers).  But when used as a special selector in interpreted code the receiver does still need validating.  I will fix this today.  Thanks for spotting my my mistake!

I wondering how much impact this optimization produces? Is it still relevant in Spur VM?
Reply | Threaded
Open this post in threaded view
|

Re: VM Maker: VMMaker.oscog-eem.2222.mcz

Eliot Miranda-2
 
Hi Denis,

On Mon, Jun 5, 2017 at 10:06 AM, Denis Kudriashov <[hidden email]> wrote:
 
Hi

2017-06-05 18:46 GMT+02:00 Eliot Miranda <[hidden email]>:
Damn, you're right.  I had forgotten about the special selector case.  The non/special selector form (a send in jotted code or a perform:) validated the receiver implicitly because the problem motive method is on Number (and Points are not numbers).  But when used as a special selector in interpreted code the receiver does still need validating.  I will fix this today.  Thanks for spotting my my mistake!

I wondering how much impact this optimization produces? Is it still relevant in Spur VM?

I'm sure it won't be noticeable except on graphics intensive apps on an interpreted VM.



--
_,,,^..^,,,_
best, Eliot