2014-05-19 20:50 GMT+02:00 <commits at source.squeak.org>:

> Eliot Miranda uploaded a new version of Kernel to project The Trunk:
> http://source.squeak.org/trunk/Kernel-eem.852.mcz
>
> ==================== Summary ====================
>
> Name: Kernel-eem.852
> Author: eem
> Time: 19 May 2014, 11:49:49.951 am
> UUID: 6d0add2f-b4ae-4e90-9b0d-7c938c4f4c1c
> Ancestors: Kernel-eem.851
>
> Change scanning methods in InstructionStream and
> CompiledMethod to double-dispatch through
> CompiledMethod>>#encoderClass to provide pluggable
> multiple bytecode set support.  The sign of a method's
> header chooses which of two bytecode sets are in effect,
> choosing between PrimaryBytecodeSetEncoderClass
> (header >= 0), and SecondaryBytecodeSetEncoderClass
> (header <= 0).
>
> =============== Diff against Kernel-eem.851 ===============
>
> Item was changed:
>   ----- Method: CompiledMethod
> class>>newBytes:trailerBytes:nArgs:nTemps:nStack:nLits:primitive: (in
> category 'instance creation') -----
>   newBytes: numberOfBytes trailerBytes: trailer nArgs: nArgs nTemps:
> nTemps nStack: stackSize nLits: nLits primitive: primitiveIndex
>         "Answer an instance of me. The header is specified by the message
> +        arguments. The remaining parts are not as yet determined."
> -       arguments. The remaining parts are not as yet determined."
>         | largeBit primBits |
>         nTemps > 63 ifTrue:
>                 [^ self error: 'Cannot compile -- too many temporary
> variables'].
>         nLits > 255 ifTrue:
> +               [^ self error: 'Cannot compile -- too many literals'].
> -               [^ self error: 'Cannot compile -- too many literals
> variables'].
>         largeBit := (nTemps + stackSize) > SmallFrame ifTrue: [1] ifFalse:
> [0].
>         primBits := primitiveIndex <= 16r1FF
>                 ifTrue: [primitiveIndex]
>                 ifFalse: ["For now the high bit of primitive no. is in the
> 29th bit of header"
>                                 primitiveIndex > 16r3FF ifTrue: [self
> error: 'prim num too large'].
>                                 (primitiveIndex bitAnd: 16r1FF) +
> ((primitiveIndex bitAnd: 16r200) bitShift: 19)].
>
>         ^trailer
>                 createMethod: numberOfBytes
>                 class: self
>                 header: (nArgs bitShift: 24) +
>                                 (nTemps bitShift: 18) +
>                                 (largeBit bitShift: 17) +
>                                 (nLits bitShift: 9) +
>                                 primBits!
>
> Item was changed:
>   ----- Method: CompiledMethod
> class>>newBytes:trailerBytes:nArgs:nTemps:nStack:nLits:primitive:flag: (in
> category 'instance creation') -----
>   newBytes: numberOfBytes trailerBytes: trailer nArgs: nArgs nTemps:
> nTemps nStack: stackSize nLits: nLits primitive: primitiveIndex flag: flag
>         "Answer an instance of me. The header is specified by the message
> +        arguments. The remaining parts are not as yet determined."
> -       arguments. The remaining parts are not as yet determined."
>         | largeBit primBits flagBit |
>         nTemps > 63 ifTrue:
>                 [^ self error: 'Cannot compile -- too many temporary
> variables'].
>         nLits > 255 ifTrue:
> +               [^ self error: 'Cannot compile -- too many literals'].
> -               [^ self error: 'Cannot compile -- too many literals
> variables'].
>         largeBit := (nTemps + stackSize) > SmallFrame ifTrue: [1] ifFalse:
> [0].
>
>         "For now the high bit of the primitive no. is in a high bit of the
> header"
>         primBits := (primitiveIndex bitAnd: 16r1FF) + ((primitiveIndex
> bitAnd: 16r200) bitShift: 19).
>
>         flagBit := flag ifTrue: [ 1 ] ifFalse: [ 0 ].
>
> -       "Copy the source code trailer to the end"
>         ^trailer
>                 createMethod: numberOfBytes
>                 class: self
>                 header: (nArgs bitShift: 24) +
>                                 (nTemps bitShift: 18) +
>                                 (largeBit bitShift: 17) +
>                                 (nLits bitShift: 9) +
>                                 primBits +
>                                 (flagBit bitShift: 29)!
>
> Item was changed:
>   ----- Method: CompiledMethod>>containsBlockClosures (in category
> 'closures') -----
>   containsBlockClosures
> +       ^self scanner scanFor: self encoderClass createClosureScanBlock!
> -       ^ self scanner scanFor: [ :bc | bc = 143 "push closure bytecode" ]!
>
> Item was changed:
>   ----- Method: CompiledMethod>>header (in category 'literals') -----
>   header
>         "Answer the word containing the information about the form of the
> +        receiver and the form of the context needed to run the receiver.
> +        There are two different formats, selected by the sign bit.  These
> are
> -       receiver and the form of the context needed to run the receiver."
>
> +        Original Squeak V3:
> +               30:sign:0 29:flag 28:prim (high bit) 27-24:numArgs
> 23-18:numTemps 17:largeFrameFlag 16-9:numLits 8-0:prim (low 9 bits)
> +
> +        Alternate Bytecode Set
> +               30:sign:1 29-28:accessModifier 27-24:numArgs
> 23-18:numTemps 17:largeFrameFlag 16:hasPrimitive 15-0:numLits
> +
> +        i.e. the Alternate Bytecode Set expands the number of literals to
> 65535 by assuming a CallPrimitive bytecode."
> +
>         ^self objectAt: 1!
>
> Item was changed:
>   ----- Method: CompiledMethod>>isBlueBookCompiled (in category 'testing')
> -----
>   isBlueBookCompiled
> +       "Answer whether the receiver was compiled using the old
> Smalltalk-80 blocks
> +        compiler.  This is used to help DebuggerMethodMap choose which
> mechanisms
> +        to use to inspect (debug) activations of the receiver."
> -       "Answer whether the receiver was compiled using the closure
> compiler.
> -        This is used to help DebuggerMethodMap choose which mechanisms to
> -        use to inspect activations of the receiver.
> -        This method answers false negatives in that it only identifies
> methods
> -        that create old BlockClosures or use the new BlockClosure
> bytecodes.
> -        It cannot tell if a method which uses neither the old nor the new
> block
> -        bytecodes is compiled with the blue-book compiler or the new
> compiler.
> -        But since methods that don't create blocks have essentially the
> same
> -        code when compiled with either compiler this makes little
> difference."
>
> +       ^self encoderClass supportsClosures not!
> -       ^((InstructionStream on: self) scanFor:
> -               [:instr |
> -               (instr >= 138 and: [instr <= 143]) ifTrue: [^false].
> -               instr = 200])
> -          or: [(self hasLiteral: #blockCopy:)
> -                  and: [self messages includes: #blockCopy:]]!
>
> Item was changed:
>   ----- Method: CompiledMethod>>isClosureCompiled (in category 'testing')
> -----
>   isClosureCompiled
>         "Answer whether the receiver was compiled using the closure
> compiler.
>          This is used to help DebuggerMethodMap choose which mechanisms to
> +        use to inspect (debug) activations of the receiver."
> -        use to inspect activations of the receiver.
> -        This method answers false negatives in that it only identifies
> methods
> -        that create new BlockClosures or use the new BlockClosure
> bytecodes.
> -        But since methods that don't create blocks have essentially the
> same
> -        code when compiled with either compiler this makes little
> difference."
>
> +       ^self encoderClass supportsClosures!
> -       ^((InstructionStream on: self) scanFor: [:instr | instr >= 138
> and: [instr <= 143]])
> -          or: [(self hasLiteral: #closureCopy:copiedValues:)
> -                  and: [self messages includes:
> #closureCopy:copiedValues:]]!
>
>
Note that this change makes
ClosureCompilerTest>>#testSourceRangeAccessForBlueBookInjectInto fail
But the test itself is for pre-closure byteCodes (using EncoderForV3, or so
called Blue Book blocks with blockCopy byteCode)

In my image, I have no blue book CompiledMethod, if I believe this snippet:

CompiledMethod allInstances count: [:e |
    ((InstructionStream on: e) scanFor: [:instr | instr = 200])
           or: [(e hasLiteral: #blockCopy:)
                   and: [e messages includes: #blockCopy:]]].

Do we really want to support this old thing?
Or shall we remove this test and support of previous bytecode set
(EncoderForV3)?
Or is there a danger that some old ImageSegment bring some zombies back?
Or are these things still usefull for some boostrap voodoo?

Currently, debugging such compiled method fails with sticky debuggers...
Some bytes are rotting, we lack tests, a single person is active/fluent on
this part, so dropping support is a reasonnable option.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20140602/a618c576/attachment.htm