Hi Nicolas,

On Mon, Oct 31, 2016 at 3:38 PM, Nicolas Cellier <nicolas.cellier.aka.nice@gmail.com> wrote:

 

2016-10-31 22:42 GMT+01:00 <commits@source.squeak.org>:

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

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

Name: VMMaker.oscog-eem.1972
Author: eem
Time: 31 October 2016, 2:42:12.362675 pm
UUID: 0e6a54ad-d62f-4b69-9f0d-7b66f8350984
Ancestors: VMMaker.oscog-eem.1971

Redo fixing extB sign extension in NewsqueakV4 & SistaV1 extPushIntegerBytecode & extUnconditionalJump in interpreter and Cogit using bitShift: 8 instead of << 8.  Slang seems to generate the correct code with bitAShift:, but not with <<.

Hi Eliot,

it would have worked if you would have used bitOr: instead of +
I'd say bitOr: better fits the intention:

you want to assemble the bits, you do not really want to perform arithmetic here.

OK, but in 2's complement they do the same thing, so + sdeems just as good for me.

 

Also, the result we are after is a signed int, so consider that << is doing the right thing, it gives you a signed int.

That's not what Slang generated.  It generated a cast to usqInt.

 

The expression you propose is unsigned. It works OK in this context, because we assign immediately to a signed int.
But for a more general usage, it could deceive the inliner (for example if variable is automatically typed or eliminated...)

Well, we have signed right shifts, but no signed left shifts.  What I really want is a signed left shift but chose not to introduce e.g. <<<.  For me I would make left shift maintain the signedness of the left-hand, generating a warning if the type couldn't be determined.  That's closest to what Smalltalk does.

=============== Diff against VMMaker.oscog-eem.1971 ===============

Item was changed:
  ----- Method: SimpleStackBasedCogit>>genExtPushIntegerBytecode (in category 'bytecode generators') -----
  genExtPushIntegerBytecode
        "NewsqueakV4:   229             11100101        iiiiiiii        Push Integer #iiiiiiii (+ Extend B * 256, where bbbbbbbb = sddddddd, e.g. -32768 = i=0, a=0, s=1)
        SistaV1:                232             11101000        iiiiiiii        Push Integer #iiiiiiii (+ Extend B * 256, where bbbbbbbb = sddddddd, e.g. -32768 = i=0, a=0, s=1)"
        | value |
+       value := byte1 + ((extB > 127 ifTrue: [extB - 256] ifFalse: [extB]) bitShift: 8).
-       value := byte1 + ((extB > 127 ifTrue: [extB - 256] ifFalse: [extB]) << 8).
        extB := 0.
        ^self genPushLiteral: (objectMemory integerObjectOf: value)!

Item was changed:
  ----- Method: SimpleStackBasedCogit>>genExtUnconditionalJump (in category 'bytecode generators') -----
  genExtUnconditionalJump
        "242            11110010        i i i i i i i i Jump i i i i i i i i (+ Extend B * 256, where bbbbbbbb = sddddddd, e.g. -32768 = i=0, a=0, s=1)"
        | distance target |
+       distance := byte1 + ((extB > 127 ifTrue: [extB - 256] ifFalse: [extB]) bitShift: 8).
-       distance := byte1 + ((extB > 127 ifTrue: [extB - 256] ifFalse: [extB]) << 8).
        self assert: distance = (self v4: (self generatorAt: byte0)
                                                                Long: bytecodePC
                                                                Branch: (extA ~= 0 ifTrue: [1] ifFalse: [0]) + (extB ~= 0 ifTrue: [1] ifFalse: [0])
                                                                Distance: methodObj).
        extB := 0.
        target := distance + 2 + bytecodePC.
        distance < 0 ifTrue:
                [^self genJumpBackTo: target].
        self genJumpTo: target.
        "The bytecode must be mapped since it can be either forward or backward, and
          backwards branches must be mapped. So if forward, we need to map."
        self annotateBytecode: self lastOpcode.
        ^0!

Item was changed:
  ----- Method: SimpleStackBasedCogit>>v4:Long:Branch:Distance: (in category 'span functions') -----
  v4: descriptor Long: pc Branch: nExts Distance: aMethodObj
        "242            11110010        i i i i i i i i Jump i i i i i i i i (+ Extend B * 256, where bbbbbbbb = sddddddd, e.g. -32768 = i=0, a=0, s=1)"
        <var: #descriptor type: #'BytecodeDescriptor *'>
        | extBValue |
        self assert: nExts >= 0.
        self parseV4Exts: nExts priorTo: pc in: aMethodObj into: [:ea :eb| extBValue := eb].
        ^(objectMemory fetchByte: pc + 1 ofObject: aMethodObj)
+       + (extBValue bitShift: 8)!
-       + (extBValue << 8)!

Item was changed:
  ----- Method: StackInterpreter>>extPushIntegerBytecode (in category 'stack bytecodes') -----
  extPushIntegerBytecode
        "229            11100101        i i i i i i i i Push Integer #iiiiiiii (+ Extend B * 256, where bbbbbbbb = sddddddd, e.g. -32768 = i=0, a=0, s=1)"
        | value |
+       value := self fetchByte + ((extB > 127 ifTrue: [extB - 256] ifFalse: [extB]) bitShift: 8).
-       value := self fetchByte + ((extB > 127 ifTrue: [extB - 256] ifFalse: [extB]) << 8).
        self fetchNextBytecode.
        extB := 0.
        self internalPush: (objectMemory integerObjectOf: value)!

Item was changed:
  ----- Method: StackInterpreter>>extUnconditionalJump (in category 'jump bytecodes') -----
  extUnconditionalJump
        "242            11110010        i i i i i i i i Jump i i i i i i i i (+ Extend B * 256, where bbbbbbbb = sddddddd, e.g. -32768 = i=0, a=0, s=1)"
        | byte offset |
        byte := self fetchByte.
+       offset := byte + ((extB > 127 ifTrue: [extB - 256] ifFalse: [extB]) bitShift: 8).
-       offset := byte + ((extB > 127 ifTrue: [extB - 256] ifFalse: [extB]) << 8).
        extB := 0.
        localIP := localIP + offset.
        self ifBackwardsCheckForEvents: offset.
        self fetchNextBytecode!

--

_,,,^..^,,,_

best, Eliot