The Trunk: Compiler-eem.380.mcz

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

The Trunk: Compiler-eem.380.mcz

commits-2
Eliot Miranda uploaded a new version of Compiler to project The Trunk:
http://source.squeak.org/trunk/Compiler-eem.380.mcz

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

Name: Compiler-eem.380
Author: eem
Time: 20 March 2018, 3:30:10.256928 pm
UUID: 3133d60f-54b2-410e-92ae-ef5cc782ab9c
Ancestors: Compiler-eem.379

Rename Encoder's addedSelectorAndMethodClassLiterals, a name which is now misleading given the new full block support, to addedExtraLiterals.  Requires Compiler-eem.379.

=============== Diff against Compiler-eem.379 ===============

Item was changed:
  ----- Method: BytecodeEncoder>>allLiteralsForBlockMethod (in category 'results') -----
  allLiteralsForBlockMethod
+ addedExtraLiterals ifFalse:
+ [addedExtraLiterals := true.
- "N.B. addedSelectorAndMethodClassLiterals is a misnomer.  It should be something like
- addedImplicitLiterals or addedExtraLiterals."
- addedSelectorAndMethodClassLiterals ifFalse:
- [addedSelectorAndMethodClassLiterals := true.
  "Put the optimized selectors in literals so as to browse senders more easily"
  optimizedSelectors := optimizedSelectors reject: [:e| literalStream originalContents hasLiteral: e].
  optimizedSelectors isEmpty ifFalse: [
  "Use one entry per literal if enough room, else make anArray"
  literalStream position + optimizedSelectors size + 2 >= self maxNumLiterals
  ifTrue: [self litIndex: optimizedSelectors asArray]
  ifFalse: [optimizedSelectors do: [:e | self litIndex: e]]].
  "Add a slot for outerCode"
  self litIndex: nil].
  ^literalStream contents!

Item was changed:
  ----- Method: BytecodeEncoder>>resetForFullBlockGeneration (in category 'code generation') -----
  resetForFullBlockGeneration
  literalStream := WriteStream on: (Array new: 8).
+ addedExtraLiterals := false.
- addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
  optimizedSelectors := Set new!

Item was changed:
  ----- Method: BytecodeEncoder>>resetLiteralStreamForFullBlock (in category 'code generation') -----
  resetLiteralStreamForFullBlock
  literalStream := WriteStream on: (Array new: 32).
+ addedExtraLiterals := false.
- addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
  optimizedSelectors := Set new!

Item was changed:
  ParseNode subclass: #Encoder
+ instanceVariableNames: 'scopeTable nTemps supered requestor class selector literalStream selectorSet litIndSet litSet sourceRanges globalSourceRanges addedExtraLiterals optimizedSelectors cue'
- instanceVariableNames: 'scopeTable nTemps supered requestor class selector literalStream selectorSet litIndSet litSet sourceRanges globalSourceRanges addedSelectorAndMethodClassLiterals optimizedSelectors cue'
  classVariableNames: ''
  poolDictionaries: ''
  category: 'Compiler-Kernel'!
 
  !Encoder commentStamp: 'cwp 12/26/2012 23:29' prior: 0!
  I encode names and literals into tree nodes with byte codes for the compiler. Byte codes for literals are not assigned until the tree-sizing pass of the compiler, because only then is it known which literals are actually needed. I also keep track of sourceCode ranges during parsing and code generation so I can provide an inverse map for the debugger.!

Item was changed:
  ----- Method: Encoder>>allLiterals (in category 'results') -----
  allLiterals
+ addedExtraLiterals ifFalse:
+ [addedExtraLiterals := true.
- addedSelectorAndMethodClassLiterals ifFalse:
- [addedSelectorAndMethodClassLiterals := true.
  "Put the optimized selectors in literals so as to browse senders more easily"
  optimizedSelectors := optimizedSelectors reject: [:e| literalStream originalContents hasLiteral: e].
  optimizedSelectors isEmpty ifFalse: [
  "Use one entry per literal if enough room, else make anArray"
  literalStream position + optimizedSelectors size + 2 >= self maxNumLiterals
  ifTrue: [self litIndex: optimizedSelectors asArray]
  ifFalse: [optimizedSelectors do: [:e | self litIndex: e]]].
  "Add a slot for selector or MethodProperties"
  self litIndex: nil.
  self litIndex: self associationForClass].
  ^literalStream contents!

Item was changed:
  ----- Method: Encoder>>initScopeAndLiteralTables (in category 'initialize-release') -----
  initScopeAndLiteralTables
 
  scopeTable := StdVariables copy.
  litSet := StdLiterals copy.
  "comments can be left hanging on nodes from previous compilations.
  probably better than this hack fix is to create the nodes afresh on each compilation."
  scopeTable do:
  [:varNode| varNode comment: nil].
  litSet do:
  [:varNode| varNode comment: nil].
  selectorSet := StdSelectors copy.
  litIndSet := Dictionary new: 16.
  literalStream := WriteStream on: (Array new: 32).
+ addedExtraLiterals := false.
- addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
  optimizedSelectors := Set new!


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Compiler-eem.380.mcz

Tobias Pape
Hi Eliot,


loading .380  produces an error for me in #allLiterals.

I think this is because of the Encoder being used for changing the encoder?
we need a few thing here probably:
 1. remove .380
 2. a commit that _Adds_ the new inst var (iirc this is .379 already)
 3. an mcm for that (iirc this is .431.mcm)
 4. remove config .432.mcm
 5. a commit that _uses_ the new inst var
 6. an mcm for that (a _new_ .432.mcm)
 7. a commit that _Removes_ the old inst var.
 (8. maybe an mcm for that..)

Best regards
-Tobias




On 20.03.2018, at 23:30, [hidden email] wrote:

Eliot Miranda uploaded a new version of Compiler to project The Trunk:
http://source.squeak.org/trunk/Compiler-eem.380.mcz

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

Name: Compiler-eem.380
Author: eem
Time: 20 March 2018, 3:30:10.256928 pm
UUID: 3133d60f-54b2-410e-92ae-ef5cc782ab9c
Ancestors: Compiler-eem.379

Rename Encoder's addedSelectorAndMethodClassLiterals, a name which is now misleading given the new full block support, to addedExtraLiterals.  Requires Compiler-eem.379.

=============== Diff against Compiler-eem.379 ===============

Item was changed:
 ----- Method: BytecodeEncoder>>allLiteralsForBlockMethod (in category 'results') -----
 allLiteralsForBlockMethod
addedExtraLiterals ifFalse:
[addedExtraLiterals := true.
"N.B. addedSelectorAndMethodClassLiterals is a misnomer.  It should be something like
 addedImplicitLiterals or addedExtraLiterals."
addedSelectorAndMethodClassLiterals ifFalse:
[addedSelectorAndMethodClassLiterals := true.
  "Put the optimized selectors in literals so as to browse senders more easily"
  optimizedSelectors := optimizedSelectors reject: [:e| literalStream originalContents hasLiteral: e].
  optimizedSelectors isEmpty ifFalse: [
  "Use one entry per literal if enough room, else make anArray"
  literalStream position + optimizedSelectors size + 2 >= self maxNumLiterals
  ifTrue: [self litIndex: optimizedSelectors asArray]
  ifFalse: [optimizedSelectors do: [:e | self litIndex: e]]].
  "Add a slot for outerCode"
  self litIndex: nil].
  ^literalStream contents!

Item was changed:
 ----- Method: BytecodeEncoder>>resetForFullBlockGeneration (in category 'code generation') -----
 resetForFullBlockGeneration
  literalStream := WriteStream on: (Array new: 8).
addedExtraLiterals := false.
addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
  optimizedSelectors := Set new!

Item was changed:
 ----- Method: BytecodeEncoder>>resetLiteralStreamForFullBlock (in category 'code generation') -----
 resetLiteralStreamForFullBlock
  literalStream := WriteStream on: (Array new: 32).
addedExtraLiterals := false.
addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
  optimizedSelectors := Set new!

Item was changed:
 ParseNode subclass: #Encoder
instanceVariableNames: 'scopeTable nTemps supered requestor class selector literalStream selectorSet litIndSet litSet sourceRanges globalSourceRanges addedExtraLiterals optimizedSelectors cue'
instanceVariableNames: 'scopeTable nTemps supered requestor class selector literalStream selectorSet litIndSet litSet sourceRanges globalSourceRanges addedSelectorAndMethodClassLiterals optimizedSelectors cue'
  classVariableNames: ''
  poolDictionaries: ''
  category: 'Compiler-Kernel'!

 !Encoder commentStamp: 'cwp 12/26/2012 23:29' prior: 0!
 I encode names and literals into tree nodes with byte codes for the compiler. Byte codes for literals are not assigned until the tree-sizing pass of the compiler, because only then is it known which literals are actually needed. I also keep track of sourceCode ranges during parsing and code generation so I can provide an inverse map for the debugger.!

Item was changed:
 ----- Method: Encoder>>allLiterals (in category 'results') -----
 allLiterals
addedExtraLiterals ifFalse:
[addedExtraLiterals := true.
addedSelectorAndMethodClassLiterals ifFalse:
[addedSelectorAndMethodClassLiterals := true.
  "Put the optimized selectors in literals so as to browse senders more easily"
  optimizedSelectors := optimizedSelectors reject: [:e| literalStream originalContents hasLiteral: e].
  optimizedSelectors isEmpty ifFalse: [
  "Use one entry per literal if enough room, else make anArray"
  literalStream position + optimizedSelectors size + 2 >= self maxNumLiterals
  ifTrue: [self litIndex: optimizedSelectors asArray]
  ifFalse: [optimizedSelectors do: [:e | self litIndex: e]]].
  "Add a slot for selector or MethodProperties"
  self litIndex: nil.
  self litIndex: self associationForClass].
  ^literalStream contents!

Item was changed:
 ----- Method: Encoder>>initScopeAndLiteralTables (in category 'initialize-release') -----
 initScopeAndLiteralTables

  scopeTable := StdVariables copy.
  litSet := StdLiterals copy.
  "comments can be left hanging on nodes from previous compilations.
   probably better than this hack fix is to create the nodes afresh on each compilation."
  scopeTable do:
  [:varNode| varNode comment: nil].
  litSet do:
  [:varNode| varNode comment: nil].
  selectorSet := StdSelectors copy.
  litIndSet := Dictionary new: 16.
  literalStream := WriteStream on: (Array new: 32).
addedExtraLiterals := false.
addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
  optimizedSelectors := Set new!





Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Compiler-eem.380.mcz

Eliot Miranda-2
Hi Tobias,

as the commit comments says, you must first load Compiler-eem.379.  The update steam contains an update for it.  And its commit comment explains the issue:

Name: Compiler-eem.379
Author: eem
Time: 20 March 2018, 3:27:27.12646 pm
UUID: b3856f24-9d98-478a-936f-c6d24d667be4
Ancestors: Compiler-eem.378

Add initialization of the Undeclared variable addedExtraLiterals which is soon to be a replacement for Encoder's addedSelectorAndMethodClassLiterals, a name which is now misleading given the new full block support.  By adding the initialization of the Undeclared variable the compiler is not broken as the instance variable is renamed and Encoder's methods are recompiled.


_,,,^..^,,,_ (phone)

On Mar 23, 2018, at 1:20 AM, Tobias Pape <[hidden email]> wrote:

Hi Eliot,


loading .380  produces an error for me in #allLiterals.

I think this is because of the Encoder being used for changing the encoder?
we need a few thing here probably:
 1. remove .380
 2. a commit that _Adds_ the new inst var (iirc this is .379 already)
 3. an mcm for that (iirc this is .431.mcm)
 4. remove config .432.mcm
 5. a commit that _uses_ the new inst var
 6. an mcm for that (a _new_ .432.mcm)
 7. a commit that _Removes_ the old inst var.
 (8. maybe an mcm for that..)

Best regards
-Tobias



<Bildschirmfoto 2018-03-23 um 09.02.37.PNG>
On 20.03.2018, at 23:30, [hidden email] wrote:

Eliot Miranda uploaded a new version of Compiler to project The Trunk:
http://source.squeak.org/trunk/Compiler-eem.380.mcz

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

Name: Compiler-eem.380
Author: eem
Time: 20 March 2018, 3:30:10.256928 pm
UUID: 3133d60f-54b2-410e-92ae-ef5cc782ab9c
Ancestors: Compiler-eem.379

Rename Encoder's addedSelectorAndMethodClassLiterals, a name which is now misleading given the new full block support, to addedExtraLiterals.  Requires Compiler-eem.379.

=============== Diff against Compiler-eem.379 ===============

Item was changed:
 ----- Method: BytecodeEncoder>>allLiteralsForBlockMethod (in category 'results') -----
 allLiteralsForBlockMethod
addedExtraLiterals ifFalse:
[addedExtraLiterals := true.
"N.B. addedSelectorAndMethodClassLiterals is a misnomer.  It should be something like
 addedImplicitLiterals or addedExtraLiterals."
addedSelectorAndMethodClassLiterals ifFalse:
[addedSelectorAndMethodClassLiterals := true.
  "Put the optimized selectors in literals so as to browse senders more easily"
  optimizedSelectors := optimizedSelectors reject: [:e| literalStream originalContents hasLiteral: e].
  optimizedSelectors isEmpty ifFalse: [
  "Use one entry per literal if enough room, else make anArray"
  literalStream position + optimizedSelectors size + 2 >= self maxNumLiterals
  ifTrue: [self litIndex: optimizedSelectors asArray]
  ifFalse: [optimizedSelectors do: [:e | self litIndex: e]]].
  "Add a slot for outerCode"
  self litIndex: nil].
  ^literalStream contents!

Item was changed:
 ----- Method: BytecodeEncoder>>resetForFullBlockGeneration (in category 'code generation') -----
 resetForFullBlockGeneration
  literalStream := WriteStream on: (Array new: 8).
addedExtraLiterals := false.
addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
  optimizedSelectors := Set new!

Item was changed:
 ----- Method: BytecodeEncoder>>resetLiteralStreamForFullBlock (in category 'code generation') -----
 resetLiteralStreamForFullBlock
  literalStream := WriteStream on: (Array new: 32).
addedExtraLiterals := false.
addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
  optimizedSelectors := Set new!

Item was changed:
 ParseNode subclass: #Encoder
instanceVariableNames: 'scopeTable nTemps supered requestor class selector literalStream selectorSet litIndSet litSet sourceRanges globalSourceRanges addedExtraLiterals optimizedSelectors cue'
instanceVariableNames: 'scopeTable nTemps supered requestor class selector literalStream selectorSet litIndSet litSet sourceRanges globalSourceRanges addedSelectorAndMethodClassLiterals optimizedSelectors cue'
  classVariableNames: ''
  poolDictionaries: ''
  category: 'Compiler-Kernel'!

 !Encoder commentStamp: 'cwp 12/26/2012 23:29' prior: 0!
 I encode names and literals into tree nodes with byte codes for the compiler. Byte codes for literals are not assigned until the tree-sizing pass of the compiler, because only then is it known which literals are actually needed. I also keep track of sourceCode ranges during parsing and code generation so I can provide an inverse map for the debugger.!

Item was changed:
 ----- Method: Encoder>>allLiterals (in category 'results') -----
 allLiterals
addedExtraLiterals ifFalse:
[addedExtraLiterals := true.
addedSelectorAndMethodClassLiterals ifFalse:
[addedSelectorAndMethodClassLiterals := true.
  "Put the optimized selectors in literals so as to browse senders more easily"
  optimizedSelectors := optimizedSelectors reject: [:e| literalStream originalContents hasLiteral: e].
  optimizedSelectors isEmpty ifFalse: [
  "Use one entry per literal if enough room, else make anArray"
  literalStream position + optimizedSelectors size + 2 >= self maxNumLiterals
  ifTrue: [self litIndex: optimizedSelectors asArray]
  ifFalse: [optimizedSelectors do: [:e | self litIndex: e]]].
  "Add a slot for selector or MethodProperties"
  self litIndex: nil.
  self litIndex: self associationForClass].
  ^literalStream contents!

Item was changed:
 ----- Method: Encoder>>initScopeAndLiteralTables (in category 'initialize-release') -----
 initScopeAndLiteralTables

  scopeTable := StdVariables copy.
  litSet := StdLiterals copy.
  "comments can be left hanging on nodes from previous compilations.
   probably better than this hack fix is to create the nodes afresh on each compilation."
  scopeTable do:
  [:varNode| varNode comment: nil].
  litSet do:
  [:varNode| varNode comment: nil].
  selectorSet := StdSelectors copy.
  litIndSet := Dictionary new: 16.
  literalStream := WriteStream on: (Array new: 32).
addedExtraLiterals := false.
addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
  optimizedSelectors := Set new!






Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Compiler-eem.380.mcz

Tobias Pape
HI Eliot,

> On 23.03.2018, at 14:55, Eliot Miranda <[hidden email]> wrote:
>
> Hi Tobias,
>
> as the commit comments says, you must first load Compiler-eem.379.  The update steam contains an update for it.  And its commit comment explains the issue:

Yes, I have 379 loaded,
no it does not help.

Reason: The class is reshaped before its methods are recompiled, hence 'addedSelectorAndMethodClassLiterals' is gone and nilled and throws a MustBeBoolean upon next use, wich is any kind of compilation, so the recompilation fails.

Best regards
        -Tobias



>
> Name: Compiler-eem.379
> Author: eem
> Time: 20 March 2018, 3:27:27.12646 pm
> UUID: b3856f24-9d98-478a-936f-c6d24d667be4
> Ancestors: Compiler-eem.378
>
> Add initialization of the Undeclared variable addedExtraLiterals which is soon to be a replacement for Encoder's addedSelectorAndMethodClassLiterals, a name which is now misleading given the new full block support.  By adding the initialization of the Undeclared variable the compiler is not broken as the instance variable is renamed and Encoder's methods are recompiled.
>
> _,,,^..^,,,_ (phone)
>
> On Mar 23, 2018, at 1:20 AM, Tobias Pape <[hidden email]> wrote:
>
>> Hi Eliot,
>>
>>
>> loading .380  produces an error for me in #allLiterals.
>>
>> I think this is because of the Encoder being used for changing the encoder?
>> we need a few thing here probably:
>>  1. remove .380
>>  2. a commit that _Adds_ the new inst var (iirc this is .379 already)
>>  3. an mcm for that (iirc this is .431.mcm)
>>  4. remove config .432.mcm
>>  5. a commit that _uses_ the new inst var
>>  6. an mcm for that (a _new_ .432.mcm)
>>  7. a commit that _Removes_ the old inst var.
>>  (8. maybe an mcm for that..)
>>
>> Best regards
>> -Tobias
>>
>>
>>
>> <Bildschirmfoto 2018-03-23 um 09.02.37.PNG>
>>> On 20.03.2018, at 23:30, [hidden email] wrote:
>>>
>>> Eliot Miranda uploaded a new version of Compiler to project The Trunk:
>>> http://source.squeak.org/trunk/Compiler-eem.380.mcz
>>>
>>> ==================== Summary ====================
>>>
>>> Name: Compiler-eem.380
>>> Author: eem
>>> Time: 20 March 2018, 3:30:10.256928 pm
>>> UUID: 3133d60f-54b2-410e-92ae-ef5cc782ab9c
>>> Ancestors: Compiler-eem.379
>>>
>>> Rename Encoder's addedSelectorAndMethodClassLiterals, a name which is now misleading given the new full block support, to addedExtraLiterals.  Requires Compiler-eem.379.
>>>
>>> =============== Diff against Compiler-eem.379 ===============
>>>
>>> Item was changed:
>>>  ----- Method: BytecodeEncoder>>allLiteralsForBlockMethod (in category 'results') -----
>>>  allLiteralsForBlockMethod
>>> + addedExtraLiterals ifFalse:
>>> + [addedExtraLiterals := true.
>>> - "N.B. addedSelectorAndMethodClassLiterals is a misnomer.  It should be something like
>>> - addedImplicitLiterals or addedExtraLiterals."
>>> - addedSelectorAndMethodClassLiterals ifFalse:
>>> - [addedSelectorAndMethodClassLiterals := true.
>>>   "Put the optimized selectors in literals so as to browse senders more easily"
>>>   optimizedSelectors := optimizedSelectors reject: [:e| literalStream originalContents hasLiteral: e].
>>>   optimizedSelectors isEmpty ifFalse: [
>>>   "Use one entry per literal if enough room, else make anArray"
>>>   literalStream position + optimizedSelectors size + 2 >= self maxNumLiterals
>>>   ifTrue: [self litIndex: optimizedSelectors asArray]
>>>   ifFalse: [optimizedSelectors do: [:e | self litIndex: e]]].
>>>   "Add a slot for outerCode"
>>>   self litIndex: nil].
>>>   ^literalStream contents!
>>>
>>> Item was changed:
>>>  ----- Method: BytecodeEncoder>>resetForFullBlockGeneration (in category 'code generation') -----
>>>  resetForFullBlockGeneration
>>>   literalStream := WriteStream on: (Array new: 8).
>>> + addedExtraLiterals := false.
>>> - addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
>>>   optimizedSelectors := Set new!
>>>
>>> Item was changed:
>>>  ----- Method: BytecodeEncoder>>resetLiteralStreamForFullBlock (in category 'code generation') -----
>>>  resetLiteralStreamForFullBlock
>>>   literalStream := WriteStream on: (Array new: 32).
>>> + addedExtraLiterals := false.
>>> - addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
>>>   optimizedSelectors := Set new!
>>>
>>> Item was changed:
>>>  ParseNode subclass: #Encoder
>>> + instanceVariableNames: 'scopeTable nTemps supered requestor class selector literalStream selectorSet litIndSet litSet sourceRanges globalSourceRanges addedExtraLiterals optimizedSelectors cue'
>>> - instanceVariableNames: 'scopeTable nTemps supered requestor class selector literalStream selectorSet litIndSet litSet sourceRanges globalSourceRanges addedSelectorAndMethodClassLiterals optimizedSelectors cue'
>>>   classVariableNames: ''
>>>   poolDictionaries: ''
>>>   category: 'Compiler-Kernel'!
>>>
>>>  !Encoder commentStamp: 'cwp 12/26/2012 23:29' prior: 0!
>>>  I encode names and literals into tree nodes with byte codes for the compiler. Byte codes for literals are not assigned until the tree-sizing pass of the compiler, because only then is it known which literals are actually needed. I also keep track of sourceCode ranges during parsing and code generation so I can provide an inverse map for the debugger.!
>>>
>>> Item was changed:
>>>  ----- Method: Encoder>>allLiterals (in category 'results') -----
>>>  allLiterals
>>> + addedExtraLiterals ifFalse:
>>> + [addedExtraLiterals := true.
>>> - addedSelectorAndMethodClassLiterals ifFalse:
>>> - [addedSelectorAndMethodClassLiterals := true.
>>>   "Put the optimized selectors in literals so as to browse senders more easily"
>>>   optimizedSelectors := optimizedSelectors reject: [:e| literalStream originalContents hasLiteral: e].
>>>   optimizedSelectors isEmpty ifFalse: [
>>>   "Use one entry per literal if enough room, else make anArray"
>>>   literalStream position + optimizedSelectors size + 2 >= self maxNumLiterals
>>>   ifTrue: [self litIndex: optimizedSelectors asArray]
>>>   ifFalse: [optimizedSelectors do: [:e | self litIndex: e]]].
>>>   "Add a slot for selector or MethodProperties"
>>>   self litIndex: nil.
>>>   self litIndex: self associationForClass].
>>>   ^literalStream contents!
>>>
>>> Item was changed:
>>>  ----- Method: Encoder>>initScopeAndLiteralTables (in category 'initialize-release') -----
>>>  initScopeAndLiteralTables
>>>
>>>   scopeTable := StdVariables copy.
>>>   litSet := StdLiterals copy.
>>>   "comments can be left hanging on nodes from previous compilations.
>>>   probably better than this hack fix is to create the nodes afresh on each compilation."
>>>   scopeTable do:
>>>   [:varNode| varNode comment: nil].
>>>   litSet do:
>>>   [:varNode| varNode comment: nil].
>>>   selectorSet := StdSelectors copy.
>>>   litIndSet := Dictionary new: 16.
>>>   literalStream := WriteStream on: (Array new: 32).
>>> + addedExtraLiterals := false.
>>> - addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
>>>   optimizedSelectors := Set new!
>>>
>>>
>>
>>
>


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Compiler-eem.380.mcz

Eliot Miranda-2
Hi Tobias,

> On Mar 23, 2018, at 7:03 AM, Tobias Pape <[hidden email]> wrote:
>
> HI Eliot,
>
>> On 23.03.2018, at 14:55, Eliot Miranda <[hidden email]> wrote:
>>
>> Hi Tobias,
>>
>> as the commit comments says, you must first load Compiler-eem.379.  The update steam contains an update for it.  And its commit comment explains the issue:
>
> Yes, I have 379 loaded,
> no it does not help.
>
> Reason: The class is reshaped before its methods are recompiled, hence 'addedSelectorAndMethodClassLiterals' is gone and nilled and throws a MustBeBoolean upon next use, wich is any kind of compilation, so the recompilation fails.

That's strange.  I tested this multiple times (in 64-bit images) before I committed (as two file-ins), and after I committed (as an update) and saw no problems.  379 adds versions that update both variables, so addedExtraLiterals is in Undeclared and both  it and the addedSelectorAndMethodClassLiterals isn't var get assigned on encoder initialization.  Then, when Encoder gets reshaped, addedSelectorAndMethodClassLiterals is out in Undeclared, addedExtraLiterals becomes the inst var, and both get initialized.  Finally, the references to addedSelectorAndMethodClassLiterals are replaced by references to  addedExtraLiterals, and the putch is complete.

So you should find addedExtraLiterals in Undeclared, with the value true before loading 380.  Then, after the reshape, but before loading the other defs, you should find addedSelectorAndMethodClassLiterals with the value true.

The first thing to determine before we throw away 380 and try again is if anyone else is being bitten by this.  If it works for everyone else (and as I said it has worked for me) then there's something broken in your image.  If it's broken for everyone then I guess the package prologue may need to add addedSelectorAndMethodClassLiterals to Undeclared with value false before the reshape and redefinition. You could test if this would work by manually adding addedSelectorAndMethodClassLiterals to Undeclared with value false before loading 380.

So has anyone else had their system broken by Compiler-eem.380?

(And I'm really sorry Tobias; I thought I had this one right; and Bert, I think this is an example where update naps are absolutely required so it might be worth adding to the update doc; I'll take a look).

>
> Best regards
>    -Tobias
>
>
>
>>
>> Name: Compiler-eem.379
>> Author: eem
>> Time: 20 March 2018, 3:27:27.12646 pm
>> UUID: b3856f24-9d98-478a-936f-c6d24d667be4
>> Ancestors: Compiler-eem.378
>>
>> Add initialization of the Undeclared variable addedExtraLiterals which is soon to be a replacement for Encoder's addedSelectorAndMethodClassLiterals, a name which is now misleading given the new full block support.  By adding the initialization of the Undeclared variable the compiler is not broken as the instance variable is renamed and Encoder's methods are recompiled.
>>
>> _,,,^..^,,,_ (phone)
>>
>>> On Mar 23, 2018, at 1:20 AM, Tobias Pape <[hidden email]> wrote:
>>>
>>> Hi Eliot,
>>>
>>>
>>> loading .380  produces an error for me in #allLiterals.
>>>
>>> I think this is because of the Encoder being used for changing the encoder?
>>> we need a few thing here probably:
>>> 1. remove .380
>>> 2. a commit that _Adds_ the new inst var (iirc this is .379 already)
>>> 3. an mcm for that (iirc this is .431.mcm)
>>> 4. remove config .432.mcm
>>> 5. a commit that _uses_ the new inst var
>>> 6. an mcm for that (a _new_ .432.mcm)
>>> 7. a commit that _Removes_ the old inst var.
>>> (8. maybe an mcm for that..)
>>>
>>> Best regards
>>>    -Tobias
>>>
>>>
>>>
>>> <Bildschirmfoto 2018-03-23 um 09.02.37.PNG>
>>>> On 20.03.2018, at 23:30, [hidden email] wrote:
>>>>
>>>> Eliot Miranda uploaded a new version of Compiler to project The Trunk:
>>>> http://source.squeak.org/trunk/Compiler-eem.380.mcz
>>>>
>>>> ==================== Summary ====================
>>>>
>>>> Name: Compiler-eem.380
>>>> Author: eem
>>>> Time: 20 March 2018, 3:30:10.256928 pm
>>>> UUID: 3133d60f-54b2-410e-92ae-ef5cc782ab9c
>>>> Ancestors: Compiler-eem.379
>>>>
>>>> Rename Encoder's addedSelectorAndMethodClassLiterals, a name which is now misleading given the new full block support, to addedExtraLiterals.  Requires Compiler-eem.379.
>>>>
>>>> =============== Diff against Compiler-eem.379 ===============
>>>>
>>>> Item was changed:
>>>> ----- Method: BytecodeEncoder>>allLiteralsForBlockMethod (in category 'results') -----
>>>> allLiteralsForBlockMethod
>>>> +    addedExtraLiterals ifFalse:
>>>> +        [addedExtraLiterals := true.
>>>> -    "N.B. addedSelectorAndMethodClassLiterals is a misnomer.  It should be something like
>>>> -     addedImplicitLiterals or addedExtraLiterals."
>>>> -    addedSelectorAndMethodClassLiterals ifFalse:
>>>> -        [addedSelectorAndMethodClassLiterals := true.
>>>>        "Put the optimized selectors in literals so as to browse senders more easily"
>>>>        optimizedSelectors := optimizedSelectors reject: [:e| literalStream originalContents hasLiteral: e].
>>>>        optimizedSelectors isEmpty ifFalse: [
>>>>            "Use one entry per literal if enough room, else make anArray"
>>>>            literalStream position + optimizedSelectors size + 2 >= self maxNumLiterals
>>>>                ifTrue: [self litIndex: optimizedSelectors asArray]
>>>>                ifFalse: [optimizedSelectors do: [:e | self litIndex: e]]].
>>>>        "Add a slot for outerCode"
>>>>        self litIndex: nil].
>>>>    ^literalStream contents!
>>>>
>>>> Item was changed:
>>>> ----- Method: BytecodeEncoder>>resetForFullBlockGeneration (in category 'code generation') -----
>>>> resetForFullBlockGeneration
>>>>    literalStream := WriteStream on: (Array new: 8).
>>>> +    addedExtraLiterals := false.
>>>> -    addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
>>>>    optimizedSelectors := Set new!
>>>>
>>>> Item was changed:
>>>> ----- Method: BytecodeEncoder>>resetLiteralStreamForFullBlock (in category 'code generation') -----
>>>> resetLiteralStreamForFullBlock
>>>>    literalStream := WriteStream on: (Array new: 32).
>>>> +    addedExtraLiterals := false.
>>>> -    addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
>>>>    optimizedSelectors := Set new!
>>>>
>>>> Item was changed:
>>>> ParseNode subclass: #Encoder
>>>> +    instanceVariableNames: 'scopeTable nTemps supered requestor class selector literalStream selectorSet litIndSet litSet sourceRanges globalSourceRanges addedExtraLiterals optimizedSelectors cue'
>>>> -    instanceVariableNames: 'scopeTable nTemps supered requestor class selector literalStream selectorSet litIndSet litSet sourceRanges globalSourceRanges addedSelectorAndMethodClassLiterals optimizedSelectors cue'
>>>>    classVariableNames: ''
>>>>    poolDictionaries: ''
>>>>    category: 'Compiler-Kernel'!
>>>>
>>>> !Encoder commentStamp: 'cwp 12/26/2012 23:29' prior: 0!
>>>> I encode names and literals into tree nodes with byte codes for the compiler. Byte codes for literals are not assigned until the tree-sizing pass of the compiler, because only then is it known which literals are actually needed. I also keep track of sourceCode ranges during parsing and code generation so I can provide an inverse map for the debugger.!
>>>>
>>>> Item was changed:
>>>> ----- Method: Encoder>>allLiterals (in category 'results') -----
>>>> allLiterals
>>>> +    addedExtraLiterals ifFalse:
>>>> +        [addedExtraLiterals := true.
>>>> -    addedSelectorAndMethodClassLiterals ifFalse:
>>>> -        [addedSelectorAndMethodClassLiterals := true.
>>>>        "Put the optimized selectors in literals so as to browse senders more easily"
>>>>        optimizedSelectors := optimizedSelectors reject: [:e| literalStream originalContents hasLiteral: e].
>>>>        optimizedSelectors isEmpty ifFalse: [
>>>>            "Use one entry per literal if enough room, else make anArray"
>>>>            literalStream position + optimizedSelectors size + 2 >= self maxNumLiterals
>>>>                ifTrue: [self litIndex: optimizedSelectors asArray]
>>>>                ifFalse: [optimizedSelectors do: [:e | self litIndex: e]]].
>>>>        "Add a slot for selector or MethodProperties"
>>>>        self litIndex: nil.
>>>>        self litIndex: self associationForClass].
>>>>    ^literalStream contents!
>>>>
>>>> Item was changed:
>>>> ----- Method: Encoder>>initScopeAndLiteralTables (in category 'initialize-release') -----
>>>> initScopeAndLiteralTables
>>>>
>>>>    scopeTable := StdVariables copy.
>>>>    litSet := StdLiterals copy.
>>>>    "comments can be left hanging on nodes from previous compilations.
>>>>     probably better than this hack fix is to create the nodes afresh on each compilation."
>>>>    scopeTable do:
>>>>        [:varNode| varNode comment: nil].
>>>>    litSet do:
>>>>        [:varNode| varNode comment: nil].
>>>>    selectorSet := StdSelectors copy.
>>>>    litIndSet := Dictionary new: 16.
>>>>    literalStream := WriteStream on: (Array new: 32).
>>>> +    addedExtraLiterals := false.
>>>> -    addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
>>>>    optimizedSelectors := Set new!
>>>>
>>>>
>>>
>>>
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Compiler-eem.380.mcz

Tobias Pape
Heho

On 23.03.2018, at 15:26, Eliot Miranda <[hidden email]> wrote:

Hi Tobias,

On Mar 23, 2018, at 7:03 AM, Tobias Pape <[hidden email]> wrote:

HI Eliot,

On 23.03.2018, at 14:55, Eliot Miranda <[hidden email]> wrote:

Hi Tobias,

as the commit comments says, you must first load Compiler-eem.379.  The update steam contains an update for it.  And its commit comment explains the issue:

Yes, I have 379 loaded,
no it does not help.

Reason: The class is reshaped before its methods are recompiled, hence 'addedSelectorAndMethodClassLiterals' is gone and nilled and throws a MustBeBoolean upon next use, wich is any kind of compilation, so the recompilation fails.

That's strange.  I tested this multiple times (in 64-bit images) before I committed (as two file-ins), and after I committed (as an update) and saw no problems.  379 adds versions that update both variables, so addedExtraLiterals is in Undeclared and both  it and the addedSelectorAndMethodClassLiterals isn't var get assigned on encoder initialization.  Then, when Encoder gets reshaped, addedSelectorAndMethodClassLiterals is out in Undeclared, addedExtraLiterals becomes the inst var, and both get initialized.  Finally, the references to addedSelectorAndMethodClassLiterals are replaced by references to  addedExtraLiterals, and the putch is complete.

So you should find addedExtraLiterals in Undeclared, with the value true before loading 380.  

Yes: 
===
Undeclared "a WeakIdentityDictionary(#addedExtraLiterals->false )"
(Encoder >> #allLiterals) "first bytecode is pushRcvr: 14, aka addedSelectorAndMethodClassLiterals)
===
and " addedSelectorAndMethodClassLiterals := addedExtraLiterals := false." is present.

The patch also looks good (especially , change allLiterals before the class reshape)


Yet, MCPackageLoader>>basicLoad changes that and pulls everything except the method defs to the front
""Pass 1: Load everything but the methods,  which are collected in methodAdditions.""


Then, after the reshape, but before loading the other defs, you should find addedSelectorAndMethodClassLiterals with the value true.

And, yes, After the step 1, undeclareds are as you expected:

Undeclared  "a WeakIdentityDictionary(#addedExtraLiterals->false #addedSelectorAndMethodClassLiterals->true )"

And Encoder>>allLiterals has the expected literal for the undeclared var.

=-=-=-==
And now I was able to step though everything just fine (after it DIDN'T work four times this morning), I feel a bit like a moron -.-'

Sorry Eliot, everything seems fine *sigh*


CARRY ON FOLKS NOTHNG TO SEE KEEP WALKING … 

best regards
-tobias



The first thing to determine before we throw away 380 and try again is if anyone else is being bitten by this.  If it works for everyone else (and as I said it has worked for me) then there's something broken in your image.  If it's broken for everyone then I guess the package prologue may need to add addedSelectorAndMethodClassLiterals to Undeclared with value false before the reshape and redefinition. You could test if this would work by manually adding addedSelectorAndMethodClassLiterals to Undeclared with value false before loading 380.

So has anyone else had their system broken by Compiler-eem.380?

(And I'm really sorry Tobias; I thought I had this one right; and Bert, I think this is an example where update naps are absolutely required so it might be worth adding to the update doc; I'll take a look).


Best regards
  -Tobias




Name: Compiler-eem.379
Author: eem
Time: 20 March 2018, 3:27:27.12646 pm
UUID: b3856f24-9d98-478a-936f-c6d24d667be4
Ancestors: Compiler-eem.378

Add initialization of the Undeclared variable addedExtraLiterals which is soon to be a replacement for Encoder's addedSelectorAndMethodClassLiterals, a name which is now misleading given the new full block support.  By adding the initialization of the Undeclared variable the compiler is not broken as the instance variable is renamed and Encoder's methods are recompiled.

_,,,^..^,,,_ (phone)

On Mar 23, 2018, at 1:20 AM, Tobias Pape <[hidden email]> wrote:

Hi Eliot,


loading .380  produces an error for me in #allLiterals.

I think this is because of the Encoder being used for changing the encoder?
we need a few thing here probably:
1. remove .380
2. a commit that _Adds_ the new inst var (iirc this is .379 already)
3. an mcm for that (iirc this is .431.mcm)
4. remove config .432.mcm
5. a commit that _uses_ the new inst var
6. an mcm for that (a _new_ .432.mcm)
7. a commit that _Removes_ the old inst var.
(8. maybe an mcm for that..)

Best regards
  -Tobias



<Bildschirmfoto 2018-03-23 um 09.02.37.PNG>
On 20.03.2018, at 23:30, [hidden email] wrote:

Eliot Miranda uploaded a new version of Compiler to project The Trunk:
http://source.squeak.org/trunk/Compiler-eem.380.mcz

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

Name: Compiler-eem.380
Author: eem
Time: 20 March 2018, 3:30:10.256928 pm
UUID: 3133d60f-54b2-410e-92ae-ef5cc782ab9c
Ancestors: Compiler-eem.379

Rename Encoder's addedSelectorAndMethodClassLiterals, a name which is now misleading given the new full block support, to addedExtraLiterals.  Requires Compiler-eem.379.

=============== Diff against Compiler-eem.379 ===============

Item was changed:
----- Method: BytecodeEncoder>>allLiteralsForBlockMethod (in category 'results') -----
allLiteralsForBlockMethod
+    addedExtraLiterals ifFalse:
+        [addedExtraLiterals := true.
-    "N.B. addedSelectorAndMethodClassLiterals is a misnomer.  It should be something like
-     addedImplicitLiterals or addedExtraLiterals."
-    addedSelectorAndMethodClassLiterals ifFalse:
-        [addedSelectorAndMethodClassLiterals := true.
      "Put the optimized selectors in literals so as to browse senders more easily"
      optimizedSelectors := optimizedSelectors reject: [:e| literalStream originalContents hasLiteral: e].
      optimizedSelectors isEmpty ifFalse: [
          "Use one entry per literal if enough room, else make anArray"
          literalStream position + optimizedSelectors size + 2 >= self maxNumLiterals
              ifTrue: [self litIndex: optimizedSelectors asArray]
              ifFalse: [optimizedSelectors do: [:e | self litIndex: e]]].
      "Add a slot for outerCode"
      self litIndex: nil].
  ^literalStream contents!

Item was changed:
----- Method: BytecodeEncoder>>resetForFullBlockGeneration (in category 'code generation') -----
resetForFullBlockGeneration
  literalStream := WriteStream on: (Array new: 8).
+    addedExtraLiterals := false.
-    addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
  optimizedSelectors := Set new!

Item was changed:
----- Method: BytecodeEncoder>>resetLiteralStreamForFullBlock (in category 'code generation') -----
resetLiteralStreamForFullBlock
  literalStream := WriteStream on: (Array new: 32).
+    addedExtraLiterals := false.
-    addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
  optimizedSelectors := Set new!

Item was changed:
ParseNode subclass: #Encoder
+    instanceVariableNames: 'scopeTable nTemps supered requestor class selector literalStream selectorSet litIndSet litSet sourceRanges globalSourceRanges addedExtraLiterals optimizedSelectors cue'
-    instanceVariableNames: 'scopeTable nTemps supered requestor class selector literalStream selectorSet litIndSet litSet sourceRanges globalSourceRanges addedSelectorAndMethodClassLiterals optimizedSelectors cue'
  classVariableNames: ''
  poolDictionaries: ''
  category: 'Compiler-Kernel'!

!Encoder commentStamp: 'cwp 12/26/2012 23:29' prior: 0!
I encode names and literals into tree nodes with byte codes for the compiler. Byte codes for literals are not assigned until the tree-sizing pass of the compiler, because only then is it known which literals are actually needed. I also keep track of sourceCode ranges during parsing and code generation so I can provide an inverse map for the debugger.!

Item was changed:
----- Method: Encoder>>allLiterals (in category 'results') -----
allLiterals
+    addedExtraLiterals ifFalse:
+        [addedExtraLiterals := true.
-    addedSelectorAndMethodClassLiterals ifFalse:
-        [addedSelectorAndMethodClassLiterals := true.
      "Put the optimized selectors in literals so as to browse senders more easily"
      optimizedSelectors := optimizedSelectors reject: [:e| literalStream originalContents hasLiteral: e].
      optimizedSelectors isEmpty ifFalse: [
          "Use one entry per literal if enough room, else make anArray"
          literalStream position + optimizedSelectors size + 2 >= self maxNumLiterals
              ifTrue: [self litIndex: optimizedSelectors asArray]
              ifFalse: [optimizedSelectors do: [:e | self litIndex: e]]].
      "Add a slot for selector or MethodProperties"
      self litIndex: nil.
      self litIndex: self associationForClass].
  ^literalStream contents!

Item was changed:
----- Method: Encoder>>initScopeAndLiteralTables (in category 'initialize-release') -----
initScopeAndLiteralTables

  scopeTable := StdVariables copy.
  litSet := StdLiterals copy.
  "comments can be left hanging on nodes from previous compilations.
   probably better than this hack fix is to create the nodes afresh on each compilation."
  scopeTable do:
      [:varNode| varNode comment: nil].
  litSet do:
      [:varNode| varNode comment: nil].
  selectorSet := StdSelectors copy.
  litIndSet := Dictionary new: 16.
  literalStream := WriteStream on: (Array new: 32).
+    addedExtraLiterals := false.
-    addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
  optimizedSelectors := Set new!











Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Compiler-eem.380.mcz

Eliot Miranda-2
Hi Tobias,

On Mar 23, 2018, at 7:56 AM, Tobias Pape <[hidden email]> wrote:

Heho

On 23.03.2018, at 15:26, Eliot Miranda <[hidden email]> wrote:

Hi Tobias,

On Mar 23, 2018, at 7:03 AM, Tobias Pape <[hidden email]> wrote:

HI Eliot,

On 23.03.2018, at 14:55, Eliot Miranda <[hidden email]> wrote:

Hi Tobias,

as the commit comments says, you must first load Compiler-eem.379.  The update steam contains an update for it.  And its commit comment explains the issue:

Yes, I have 379 loaded,
no it does not help.

Reason: The class is reshaped before its methods are recompiled, hence 'addedSelectorAndMethodClassLiterals' is gone and nilled and throws a MustBeBoolean upon next use, wich is any kind of compilation, so the recompilation fails.

That's strange.  I tested this multiple times (in 64-bit images) before I committed (as two file-ins), and after I committed (as an update) and saw no problems.  379 adds versions that update both variables, so addedExtraLiterals is in Undeclared and both  it and the addedSelectorAndMethodClassLiterals isn't var get assigned on encoder initialization.  Then, when Encoder gets reshaped, addedSelectorAndMethodClassLiterals is out in Undeclared, addedExtraLiterals becomes the inst var, and both get initialized.  Finally, the references to addedSelectorAndMethodClassLiterals are replaced by references to  addedExtraLiterals, and the putch is complete.

So you should find addedExtraLiterals in Undeclared, with the value true before loading 380.  

Yes: 
===
Undeclared "a WeakIdentityDictionary(#addedExtraLiterals->false )"
(Encoder >> #allLiterals) "first bytecode is pushRcvr: 14, aka addedSelectorAndMethodClassLiterals)
===
and " addedSelectorAndMethodClassLiterals := addedExtraLiterals := false." is present.

The patch also looks good (especially , change allLiterals before the class reshape)
<Bildschirmfoto 2018-03-23 um 15.44.47.PNG>


Yet, MCPackageLoader>>basicLoad changes that and pulls everything except the method defs to the front
""Pass 1: Load everything but the methods,  which are collected in methodAdditions.""


Then, after the reshape, but before loading the other defs, you should find addedSelectorAndMethodClassLiterals with the value true.

And, yes, After the step 1, undeclareds are as you expected:

Undeclared  "a WeakIdentityDictionary(#addedExtraLiterals->false #addedSelectorAndMethodClassLiterals->true )"

And Encoder>>allLiterals has the expected literal for the undeclared var.

=-=-=-==
And now I was able to step though everything just fine (after it DIDN'T work four times this morning), I feel a bit like a moron -.-'

Sorry Eliot, everything seems fine *sigh*


CARRY ON FOLKS NOTHNG TO SEE KEEP WALKING … 

Hang on !! :-). Stepping through isn't a solution.  If you use, for example, Update Squeak from the Squeak menu does it work or not?  I'd still like to understand d why it broke for you...


best regards
-tobias



The first thing to determine before we throw away 380 and try again is if anyone else is being bitten by this.  If it works for everyone else (and as I said it has worked for me) then there's something broken in your image.  If it's broken for everyone then I guess the package prologue may need to add addedSelectorAndMethodClassLiterals to Undeclared with value false before the reshape and redefinition. You could test if this would work by manually adding addedSelectorAndMethodClassLiterals to Undeclared with value false before loading 380.

So has anyone else had their system broken by Compiler-eem.380?

(And I'm really sorry Tobias; I thought I had this one right; and Bert, I think this is an example where update naps are absolutely required so it might be worth adding to the update doc; I'll take a look).


Best regards
  -Tobias




Name: Compiler-eem.379
Author: eem
Time: 20 March 2018, 3:27:27.12646 pm
UUID: b3856f24-9d98-478a-936f-c6d24d667be4
Ancestors: Compiler-eem.378

Add initialization of the Undeclared variable addedExtraLiterals which is soon to be a replacement for Encoder's addedSelectorAndMethodClassLiterals, a name which is now misleading given the new full block support.  By adding the initialization of the Undeclared variable the compiler is not broken as the instance variable is renamed and Encoder's methods are recompiled.

_,,,^..^,,,_ (phone)

On Mar 23, 2018, at 1:20 AM, Tobias Pape <[hidden email]> wrote:

Hi Eliot,


loading .380  produces an error for me in #allLiterals.

I think this is because of the Encoder being used for changing the encoder?
we need a few thing here probably:
1. remove .380
2. a commit that _Adds_ the new inst var (iirc this is .379 already)
3. an mcm for that (iirc this is .431.mcm)
4. remove config .432.mcm
5. a commit that _uses_ the new inst var
6. an mcm for that (a _new_ .432.mcm)
7. a commit that _Removes_ the old inst var.
(8. maybe an mcm for that..)

Best regards
  -Tobias



<Bildschirmfoto 2018-03-23 um 09.02.37.PNG>
On 20.03.2018, at 23:30, [hidden email] wrote:

Eliot Miranda uploaded a new version of Compiler to project The Trunk:
http://source.squeak.org/trunk/Compiler-eem.380.mcz

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

Name: Compiler-eem.380
Author: eem
Time: 20 March 2018, 3:30:10.256928 pm
UUID: 3133d60f-54b2-410e-92ae-ef5cc782ab9c
Ancestors: Compiler-eem.379

Rename Encoder's addedSelectorAndMethodClassLiterals, a name which is now misleading given the new full block support, to addedExtraLiterals.  Requires Compiler-eem.379.

=============== Diff against Compiler-eem.379 ===============

Item was changed:
----- Method: BytecodeEncoder>>allLiteralsForBlockMethod (in category 'results') -----
allLiteralsForBlockMethod
+    addedExtraLiterals ifFalse:
+        [addedExtraLiterals := true.
-    "N.B. addedSelectorAndMethodClassLiterals is a misnomer.  It should be something like
-     addedImplicitLiterals or addedExtraLiterals."
-    addedSelectorAndMethodClassLiterals ifFalse:
-        [addedSelectorAndMethodClassLiterals := true.
      "Put the optimized selectors in literals so as to browse senders more easily"
      optimizedSelectors := optimizedSelectors reject: [:e| literalStream originalContents hasLiteral: e].
      optimizedSelectors isEmpty ifFalse: [
          "Use one entry per literal if enough room, else make anArray"
          literalStream position + optimizedSelectors size + 2 >= self maxNumLiterals
              ifTrue: [self litIndex: optimizedSelectors asArray]
              ifFalse: [optimizedSelectors do: [:e | self litIndex: e]]].
      "Add a slot for outerCode"
      self litIndex: nil].
  ^literalStream contents!

Item was changed:
----- Method: BytecodeEncoder>>resetForFullBlockGeneration (in category 'code generation') -----
resetForFullBlockGeneration
  literalStream := WriteStream on: (Array new: 8).
+    addedExtraLiterals := false.
-    addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
  optimizedSelectors := Set new!

Item was changed:
----- Method: BytecodeEncoder>>resetLiteralStreamForFullBlock (in category 'code generation') -----
resetLiteralStreamForFullBlock
  literalStream := WriteStream on: (Array new: 32).
+    addedExtraLiterals := false.
-    addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
  optimizedSelectors := Set new!

Item was changed:
ParseNode subclass: #Encoder
+    instanceVariableNames: 'scopeTable nTemps supered requestor class selector literalStream selectorSet litIndSet litSet sourceRanges globalSourceRanges addedExtraLiterals optimizedSelectors cue'
-    instanceVariableNames: 'scopeTable nTemps supered requestor class selector literalStream selectorSet litIndSet litSet sourceRanges globalSourceRanges addedSelectorAndMethodClassLiterals optimizedSelectors cue'
  classVariableNames: ''
  poolDictionaries: ''
  category: 'Compiler-Kernel'!

!Encoder commentStamp: 'cwp 12/26/2012 23:29' prior: 0!
I encode names and literals into tree nodes with byte codes for the compiler. Byte codes for literals are not assigned until the tree-sizing pass of the compiler, because only then is it known which literals are actually needed. I also keep track of sourceCode ranges during parsing and code generation so I can provide an inverse map for the debugger.!

Item was changed:
----- Method: Encoder>>allLiterals (in category 'results') -----
allLiterals
+    addedExtraLiterals ifFalse:
+        [addedExtraLiterals := true.
-    addedSelectorAndMethodClassLiterals ifFalse:
-        [addedSelectorAndMethodClassLiterals := true.
      "Put the optimized selectors in literals so as to browse senders more easily"
      optimizedSelectors := optimizedSelectors reject: [:e| literalStream originalContents hasLiteral: e].
      optimizedSelectors isEmpty ifFalse: [
          "Use one entry per literal if enough room, else make anArray"
          literalStream position + optimizedSelectors size + 2 >= self maxNumLiterals
              ifTrue: [self litIndex: optimizedSelectors asArray]
              ifFalse: [optimizedSelectors do: [:e | self litIndex: e]]].
      "Add a slot for selector or MethodProperties"
      self litIndex: nil.
      self litIndex: self associationForClass].
  ^literalStream contents!

Item was changed:
----- Method: Encoder>>initScopeAndLiteralTables (in category 'initialize-release') -----
initScopeAndLiteralTables

  scopeTable := StdVariables copy.
  litSet := StdLiterals copy.
  "comments can be left hanging on nodes from previous compilations.
   probably better than this hack fix is to create the nodes afresh on each compilation."
  scopeTable do:
      [:varNode| varNode comment: nil].
  litSet do:
      [:varNode| varNode comment: nil].
  selectorSet := StdSelectors copy.
  litIndSet := Dictionary new: 16.
  literalStream := WriteStream on: (Array new: 32).
+    addedExtraLiterals := false.
-    addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
  optimizedSelectors := Set new!












Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Compiler-eem.380.mcz

Nicolas Cellier
Hi Tobias,
Yes, some months ago I changed Undeclared to automagically clean-up when bindings are no more used.
This was necessary to workaround some Environment bugs, and it is a nice feature anyway.
So, if you want some Undeclared binding to persist, then you must have a strong pointer on it.
IOW, there should be some class with source code refering to this undeclared binding...
From what I see, there is such reference in http://source.squeak.org/trunk/Compiler-eem.379.diff
If there is a reference to this version in the update map, then update should work (and it did for me).

2018-03-23 16:53 GMT+01:00 Eliot Miranda <[hidden email]>:
Hi Tobias,

On Mar 23, 2018, at 7:56 AM, Tobias Pape <[hidden email]> wrote:

Heho

On 23.03.2018, at 15:26, Eliot Miranda <[hidden email]> wrote:

Hi Tobias,

On Mar 23, 2018, at 7:03 AM, Tobias Pape <[hidden email]> wrote:

HI Eliot,

On 23.03.2018, at 14:55, Eliot Miranda <[hidden email]> wrote:

Hi Tobias,

as the commit comments says, you must first load Compiler-eem.379.  The update steam contains an update for it.  And its commit comment explains the issue:

Yes, I have 379 loaded,
no it does not help.

Reason: The class is reshaped before its methods are recompiled, hence 'addedSelectorAndMethodClassLiterals' is gone and nilled and throws a MustBeBoolean upon next use, wich is any kind of compilation, so the recompilation fails.

That's strange.  I tested this multiple times (in 64-bit images) before I committed (as two file-ins), and after I committed (as an update) and saw no problems.  379 adds versions that update both variables, so addedExtraLiterals is in Undeclared and both  it and the addedSelectorAndMethodClassLiterals isn't var get assigned on encoder initialization.  Then, when Encoder gets reshaped, addedSelectorAndMethodClassLiterals is out in Undeclared, addedExtraLiterals becomes the inst var, and both get initialized.  Finally, the references to addedSelectorAndMethodClassLiterals are replaced by references to  addedExtraLiterals, and the putch is complete.

So you should find addedExtraLiterals in Undeclared, with the value true before loading 380.  

Yes: 
===
Undeclared "a WeakIdentityDictionary(#addedExtraLiterals->false )"
(Encoder >> #allLiterals) "first bytecode is pushRcvr: 14, aka addedSelectorAndMethodClassLiterals)
===
and " addedSelectorAndMethodClassLiterals := addedExtraLiterals := false." is present.

The patch also looks good (especially , change allLiterals before the class reshape)
<Bildschirmfoto 2018-03-23 um 15.44.47.PNG>


Yet, MCPackageLoader>>basicLoad changes that and pulls everything except the method defs to the front
""Pass 1: Load everything but the methods,  which are collected in methodAdditions.""


Then, after the reshape, but before loading the other defs, you should find addedSelectorAndMethodClassLiterals with the value true.

And, yes, After the step 1, undeclareds are as you expected:

Undeclared  "a WeakIdentityDictionary(#addedExtraLiterals->false #addedSelectorAndMethodClassLiterals->true )"

And Encoder>>allLiterals has the expected literal for the undeclared var.

=-=-=-==
And now I was able to step though everything just fine (after it DIDN'T work four times this morning), I feel a bit like a moron -.-'

Sorry Eliot, everything seems fine *sigh*


CARRY ON FOLKS NOTHNG TO SEE KEEP WALKING … 

Hang on !! :-). Stepping through isn't a solution.  If you use, for example, Update Squeak from the Squeak menu does it work or not?  I'd still like to understand d why it broke for you...


best regards
-tobias



The first thing to determine before we throw away 380 and try again is if anyone else is being bitten by this.  If it works for everyone else (and as I said it has worked for me) then there's something broken in your image.  If it's broken for everyone then I guess the package prologue may need to add addedSelectorAndMethodClassLiterals to Undeclared with value false before the reshape and redefinition. You could test if this would work by manually adding addedSelectorAndMethodClassLiterals to Undeclared with value false before loading 380.

So has anyone else had their system broken by Compiler-eem.380?

(And I'm really sorry Tobias; I thought I had this one right; and Bert, I think this is an example where update naps are absolutely required so it might be worth adding to the update doc; I'll take a look).


Best regards
  -Tobias




Name: Compiler-eem.379
Author: eem
Time: 20 March 2018, 3:27:27.12646 pm
UUID: b3856f24-9d98-478a-936f-c6d24d667be4
Ancestors: Compiler-eem.378

Add initialization of the Undeclared variable addedExtraLiterals which is soon to be a replacement for Encoder's addedSelectorAndMethodClassLiterals, a name which is now misleading given the new full block support.  By adding the initialization of the Undeclared variable the compiler is not broken as the instance variable is renamed and Encoder's methods are recompiled.

_,,,^..^,,,_ (phone)

On Mar 23, 2018, at 1:20 AM, Tobias Pape <[hidden email]> wrote:

Hi Eliot,


loading .380  produces an error for me in #allLiterals.

I think this is because of the Encoder being used for changing the encoder?
we need a few thing here probably:
1. remove .380
2. a commit that _Adds_ the new inst var (iirc this is .379 already)
3. an mcm for that (iirc this is .431.mcm)
4. remove config .432.mcm
5. a commit that _uses_ the new inst var
6. an mcm for that (a _new_ .432.mcm)
7. a commit that _Removes_ the old inst var.
(8. maybe an mcm for that..)

Best regards
  -Tobias



<Bildschirmfoto 2018-03-23 um 09.02.37.PNG>
On 20.03.2018, at 23:30, [hidden email] wrote:

Eliot Miranda uploaded a new version of Compiler to project The Trunk:
http://source.squeak.org/trunk/Compiler-eem.380.mcz

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

Name: Compiler-eem.380
Author: eem
Time: 20 March 2018, 3:30:10.256928 pm
UUID: 3133d60f-54b2-410e-92ae-ef5cc782ab9c
Ancestors: Compiler-eem.379

Rename Encoder's addedSelectorAndMethodClassLiterals, a name which is now misleading given the new full block support, to addedExtraLiterals.  Requires Compiler-eem.379.

=============== Diff against Compiler-eem.379 ===============

Item was changed:
----- Method: BytecodeEncoder>>allLiteralsForBlockMethod (in category 'results') -----
allLiteralsForBlockMethod
+    addedExtraLiterals ifFalse:
+        [addedExtraLiterals := true.
-    "N.B. addedSelectorAndMethodClassLiterals is a misnomer.  It should be something like
-     addedImplicitLiterals or addedExtraLiterals."
-    addedSelectorAndMethodClassLiterals ifFalse:
-        [addedSelectorAndMethodClassLiterals := true.
      "Put the optimized selectors in literals so as to browse senders more easily"
      optimizedSelectors := optimizedSelectors reject: [:e| literalStream originalContents hasLiteral: e].
      optimizedSelectors isEmpty ifFalse: [
          "Use one entry per literal if enough room, else make anArray"
          literalStream position + optimizedSelectors size + 2 >= self maxNumLiterals
              ifTrue: [self litIndex: optimizedSelectors asArray]
              ifFalse: [optimizedSelectors do: [:e | self litIndex: e]]].
      "Add a slot for outerCode"
      self litIndex: nil].
  ^literalStream contents!

Item was changed:
----- Method: BytecodeEncoder>>resetForFullBlockGeneration (in category 'code generation') -----
resetForFullBlockGeneration
  literalStream := WriteStream on: (Array new: 8).
+    addedExtraLiterals := false.
-    addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
  optimizedSelectors := Set new!

Item was changed:
----- Method: BytecodeEncoder>>resetLiteralStreamForFullBlock (in category 'code generation') -----
resetLiteralStreamForFullBlock
  literalStream := WriteStream on: (Array new: 32).
+    addedExtraLiterals := false.
-    addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
  optimizedSelectors := Set new!

Item was changed:
ParseNode subclass: #Encoder
+    instanceVariableNames: 'scopeTable nTemps supered requestor class selector literalStream selectorSet litIndSet litSet sourceRanges globalSourceRanges addedExtraLiterals optimizedSelectors cue'
-    instanceVariableNames: 'scopeTable nTemps supered requestor class selector literalStream selectorSet litIndSet litSet sourceRanges globalSourceRanges addedSelectorAndMethodClassLiterals optimizedSelectors cue'
  classVariableNames: ''
  poolDictionaries: ''
  category: 'Compiler-Kernel'!

!Encoder commentStamp: 'cwp 12/26/2012 23:29' prior: 0!
I encode names and literals into tree nodes with byte codes for the compiler. Byte codes for literals are not assigned until the tree-sizing pass of the compiler, because only then is it known which literals are actually needed. I also keep track of sourceCode ranges during parsing and code generation so I can provide an inverse map for the debugger.!

Item was changed:
----- Method: Encoder>>allLiterals (in category 'results') -----
allLiterals
+    addedExtraLiterals ifFalse:
+        [addedExtraLiterals := true.
-    addedSelectorAndMethodClassLiterals ifFalse:
-        [addedSelectorAndMethodClassLiterals := true.
      "Put the optimized selectors in literals so as to browse senders more easily"
      optimizedSelectors := optimizedSelectors reject: [:e| literalStream originalContents hasLiteral: e].
      optimizedSelectors isEmpty ifFalse: [
          "Use one entry per literal if enough room, else make anArray"
          literalStream position + optimizedSelectors size + 2 >= self maxNumLiterals
              ifTrue: [self litIndex: optimizedSelectors asArray]
              ifFalse: [optimizedSelectors do: [:e | self litIndex: e]]].
      "Add a slot for selector or MethodProperties"
      self litIndex: nil.
      self litIndex: self associationForClass].
  ^literalStream contents!

Item was changed:
----- Method: Encoder>>initScopeAndLiteralTables (in category 'initialize-release') -----
initScopeAndLiteralTables

  scopeTable := StdVariables copy.
  litSet := StdLiterals copy.
  "comments can be left hanging on nodes from previous compilations.
   probably better than this hack fix is to create the nodes afresh on each compilation."
  scopeTable do:
      [:varNode| varNode comment: nil].
  litSet do:
      [:varNode| varNode comment: nil].
  selectorSet := StdSelectors copy.
  litIndSet := Dictionary new: 16.
  literalStream := WriteStream on: (Array new: 32).
+    addedExtraLiterals := false.
-    addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
  optimizedSelectors := Set new!
















Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Compiler-eem.380.mcz

Tobias Pape
In reply to this post by Eliot Miranda-2
Hi Eliot,

> On 23.03.2018, at 16:53, Eliot Miranda <[hidden email]> wrote:
>
> Hi Tobias,
>
> On Mar 23, 2018, at 7:56 AM, Tobias Pape <[hidden email]> wrote:
>
>> Heho
>>
>>> On 23.03.2018, at 15:26, Eliot Miranda <[hidden email]> wrote:
>>>
>>> Hi Tobias,
>>>
>>>> On Mar 23, 2018, at 7:03 AM, Tobias Pape <[hidden email]> wrote:
>>>>
>>>> HI Eliot,
>>>>
>>>>> On 23.03.2018, at 14:55, Eliot Miranda <[hidden email]> wrote:
>>>>>
>>>>> Hi Tobias,
>>>>>
>>>>> as the commit comments says, you must first load Compiler-eem.379.  The update steam contains an update for it.  And its commit comment explains the issue:
>>>>
>>>> Yes, I have 379 loaded,
>>>> no it does not help.
>>>>
>>>> Reason: The class is reshaped before its methods are recompiled, hence 'addedSelectorAndMethodClassLiterals' is gone and nilled and throws a MustBeBoolean upon next use, wich is any kind of compilation, so the recompilation fails.
>>>
>>> That's strange.  I tested this multiple times (in 64-bit images) before I committed (as two file-ins), and after I committed (as an update) and saw no problems.  379 adds versions that update both variables, so addedExtraLiterals is in Undeclared and both  it and the addedSelectorAndMethodClassLiterals isn't var get assigned on encoder initialization.  Then, when Encoder gets reshaped, addedSelectorAndMethodClassLiterals is out in Undeclared, addedExtraLiterals becomes the inst var, and both get initialized.  Finally, the references to addedSelectorAndMethodClassLiterals are replaced by references to  addedExtraLiterals, and the putch is complete.
>>>
>>> So you should find addedExtraLiterals in Undeclared, with the value true before loading 380.  
>>
>> Yes:
>> ===
>> Undeclared "a WeakIdentityDictionary(#addedExtraLiterals->false )"
>> (Encoder >> #allLiterals) "first bytecode is pushRcvr: 14, aka addedSelectorAndMethodClassLiterals)
>> ===
>> and " addedSelectorAndMethodClassLiterals := addedExtraLiterals := false." is present.
>>
>> The patch also looks good (especially , change allLiterals before the class reshape)
>> <Bildschirmfoto 2018-03-23 um 15.44.47.PNG>
>>
>>
>> Yet, MCPackageLoader>>basicLoad changes that and pulls everything except the method defs to the front
>> ""Pass 1: Load everything but the methods,  which are collected in methodAdditions.""
>>
>>
>>> Then, after the reshape, but before loading the other defs, you should find addedSelectorAndMethodClassLiterals with the value true.
>>
>> And, yes, After the step 1, undeclareds are as you expected:
>>
>> Undeclared  "a WeakIdentityDictionary(#addedExtraLiterals->false #addedSelectorAndMethodClassLiterals->true )"
>>
>> And Encoder>>allLiterals has the expected literal for the undeclared var.
>>
>> =-=-=-==
>> And now I was able to step though everything just fine (after it DIDN'T work four times this morning), I feel a bit like a moron -.-'
>>
>> Sorry Eliot, everything seems fine *sigh*
>>
>>
>> CARRY ON FOLKS NOTHNG TO SEE KEEP WALKING …
>
> Hang on !! :-). Stepping through isn't a solution.  If you use, for example, Update Squeak from the Squeak menu does it work or not?  I'd still like to understand d why it broke for you...

Me to. Unfortunately, the image in that state is gone. And although I resurrected one from a few hours earlier, I no longer can reproduce the issue :/

Consider it a non issue now.

Best regards
        -Tobias

>
>>
>> best regards
>> -tobias
>>
>>
>>>
>>> The first thing to determine before we throw away 380 and try again is if anyone else is being bitten by this.  If it works for everyone else (and as I said it has worked for me) then there's something broken in your image.  If it's broken for everyone then I guess the package prologue may need to add addedSelectorAndMethodClassLiterals to Undeclared with value false before the reshape and redefinition. You could test if this would work by manually adding addedSelectorAndMethodClassLiterals to Undeclared with value false before loading 380.
>>>
>>> So has anyone else had their system broken by Compiler-eem.380?
>>>
>>> (And I'm really sorry Tobias; I thought I had this one right; and Bert, I think this is an example where update naps are absolutely required so it might be worth adding to the update doc; I'll take a look).
>>>
>>>>
>>>> Best regards
>>>>   -Tobias
>>>>
>>>>
>>>>
>>>>>
>>>>> Name: Compiler-eem.379
>>>>> Author: eem
>>>>> Time: 20 March 2018, 3:27:27.12646 pm
>>>>> UUID: b3856f24-9d98-478a-936f-c6d24d667be4
>>>>> Ancestors: Compiler-eem.378
>>>>>
>>>>> Add initialization of the Undeclared variable addedExtraLiterals which is soon to be a replacement for Encoder's addedSelectorAndMethodClassLiterals, a name which is now misleading given the new full block support.  By adding the initialization of the Undeclared variable the compiler is not broken as the instance variable is renamed and Encoder's methods are recompiled.
>>>>>
>>>>> _,,,^..^,,,_ (phone)
>>>>>
>>>>>> On Mar 23, 2018, at 1:20 AM, Tobias Pape <[hidden email]> wrote:
>>>>>>
>>>>>> Hi Eliot,
>>>>>>
>>>>>>
>>>>>> loading .380  produces an error for me in #allLiterals.
>>>>>>
>>>>>> I think this is because of the Encoder being used for changing the encoder?
>>>>>> we need a few thing here probably:
>>>>>> 1. remove .380
>>>>>> 2. a commit that _Adds_ the new inst var (iirc this is .379 already)
>>>>>> 3. an mcm for that (iirc this is .431.mcm)
>>>>>> 4. remove config .432.mcm
>>>>>> 5. a commit that _uses_ the new inst var
>>>>>> 6. an mcm for that (a _new_ .432.mcm)
>>>>>> 7. a commit that _Removes_ the old inst var.
>>>>>> (8. maybe an mcm for that..)
>>>>>>
>>>>>> Best regards
>>>>>>   -Tobias
>>>>>>
>>>>>>
>>>>>>
>>>>>> <Bildschirmfoto 2018-03-23 um 09.02.37.PNG>
>>>>>>> On 20.03.2018, at 23:30, [hidden email] wrote:
>>>>>>>
>>>>>>> Eliot Miranda uploaded a new version of Compiler to project The Trunk:
>>>>>>> http://source.squeak.org/trunk/Compiler-eem.380.mcz
>>>>>>>
>>>>>>> ==================== Summary ====================
>>>>>>>
>>>>>>> Name: Compiler-eem.380
>>>>>>> Author: eem
>>>>>>> Time: 20 March 2018, 3:30:10.256928 pm
>>>>>>> UUID: 3133d60f-54b2-410e-92ae-ef5cc782ab9c
>>>>>>> Ancestors: Compiler-eem.379
>>>>>>>
>>>>>>> Rename Encoder's addedSelectorAndMethodClassLiterals, a name which is now misleading given the new full block support, to addedExtraLiterals.  Requires Compiler-eem.379.
>>>>>>>
>>>>>>> =============== Diff against Compiler-eem.379 ===============
>>>>>>>
>>>>>>> Item was changed:
>>>>>>> ----- Method: BytecodeEncoder>>allLiteralsForBlockMethod (in category 'results') -----
>>>>>>> allLiteralsForBlockMethod
>>>>>>> +    addedExtraLiterals ifFalse:
>>>>>>> +        [addedExtraLiterals := true.
>>>>>>> -    "N.B. addedSelectorAndMethodClassLiterals is a misnomer.  It should be something like
>>>>>>> -     addedImplicitLiterals or addedExtraLiterals."
>>>>>>> -    addedSelectorAndMethodClassLiterals ifFalse:
>>>>>>> -        [addedSelectorAndMethodClassLiterals := true.
>>>>>>>       "Put the optimized selectors in literals so as to browse senders more easily"
>>>>>>>       optimizedSelectors := optimizedSelectors reject: [:e| literalStream originalContents hasLiteral: e].
>>>>>>>       optimizedSelectors isEmpty ifFalse: [
>>>>>>>           "Use one entry per literal if enough room, else make anArray"
>>>>>>>           literalStream position + optimizedSelectors size + 2 >= self maxNumLiterals
>>>>>>>               ifTrue: [self litIndex: optimizedSelectors asArray]
>>>>>>>               ifFalse: [optimizedSelectors do: [:e | self litIndex: e]]].
>>>>>>>       "Add a slot for outerCode"
>>>>>>>       self litIndex: nil].
>>>>>>>   ^literalStream contents!
>>>>>>>
>>>>>>> Item was changed:
>>>>>>> ----- Method: BytecodeEncoder>>resetForFullBlockGeneration (in category 'code generation') -----
>>>>>>> resetForFullBlockGeneration
>>>>>>>   literalStream := WriteStream on: (Array new: 8).
>>>>>>> +    addedExtraLiterals := false.
>>>>>>> -    addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
>>>>>>>   optimizedSelectors := Set new!
>>>>>>>
>>>>>>> Item was changed:
>>>>>>> ----- Method: BytecodeEncoder>>resetLiteralStreamForFullBlock (in category 'code generation') -----
>>>>>>> resetLiteralStreamForFullBlock
>>>>>>>   literalStream := WriteStream on: (Array new: 32).
>>>>>>> +    addedExtraLiterals := false.
>>>>>>> -    addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
>>>>>>>   optimizedSelectors := Set new!
>>>>>>>
>>>>>>> Item was changed:
>>>>>>> ParseNode subclass: #Encoder
>>>>>>> +    instanceVariableNames: 'scopeTable nTemps supered requestor class selector literalStream selectorSet litIndSet litSet sourceRanges globalSourceRanges addedExtraLiterals optimizedSelectors cue'
>>>>>>> -    instanceVariableNames: 'scopeTable nTemps supered requestor class selector literalStream selectorSet litIndSet litSet sourceRanges globalSourceRanges addedSelectorAndMethodClassLiterals optimizedSelectors cue'
>>>>>>>   classVariableNames: ''
>>>>>>>   poolDictionaries: ''
>>>>>>>   category: 'Compiler-Kernel'!
>>>>>>>
>>>>>>> !Encoder commentStamp: 'cwp 12/26/2012 23:29' prior: 0!
>>>>>>> I encode names and literals into tree nodes with byte codes for the compiler. Byte codes for literals are not assigned until the tree-sizing pass of the compiler, because only then is it known which literals are actually needed. I also keep track of sourceCode ranges during parsing and code generation so I can provide an inverse map for the debugger.!
>>>>>>>
>>>>>>> Item was changed:
>>>>>>> ----- Method: Encoder>>allLiterals (in category 'results') -----
>>>>>>> allLiterals
>>>>>>> +    addedExtraLiterals ifFalse:
>>>>>>> +        [addedExtraLiterals := true.
>>>>>>> -    addedSelectorAndMethodClassLiterals ifFalse:
>>>>>>> -        [addedSelectorAndMethodClassLiterals := true.
>>>>>>>       "Put the optimized selectors in literals so as to browse senders more easily"
>>>>>>>       optimizedSelectors := optimizedSelectors reject: [:e| literalStream originalContents hasLiteral: e].
>>>>>>>       optimizedSelectors isEmpty ifFalse: [
>>>>>>>           "Use one entry per literal if enough room, else make anArray"
>>>>>>>           literalStream position + optimizedSelectors size + 2 >= self maxNumLiterals
>>>>>>>               ifTrue: [self litIndex: optimizedSelectors asArray]
>>>>>>>               ifFalse: [optimizedSelectors do: [:e | self litIndex: e]]].
>>>>>>>       "Add a slot for selector or MethodProperties"
>>>>>>>       self litIndex: nil.
>>>>>>>       self litIndex: self associationForClass].
>>>>>>>   ^literalStream contents!
>>>>>>>
>>>>>>> Item was changed:
>>>>>>> ----- Method: Encoder>>initScopeAndLiteralTables (in category 'initialize-release') -----
>>>>>>> initScopeAndLiteralTables
>>>>>>>
>>>>>>>   scopeTable := StdVariables copy.
>>>>>>>   litSet := StdLiterals copy.
>>>>>>>   "comments can be left hanging on nodes from previous compilations.
>>>>>>>    probably better than this hack fix is to create the nodes afresh on each compilation."
>>>>>>>   scopeTable do:
>>>>>>>       [:varNode| varNode comment: nil].
>>>>>>>   litSet do:
>>>>>>>       [:varNode| varNode comment: nil].
>>>>>>>   selectorSet := StdSelectors copy.
>>>>>>>   litIndSet := Dictionary new: 16.
>>>>>>>   literalStream := WriteStream on: (Array new: 32).
>>>>>>> +    addedExtraLiterals := false.
>>>>>>> -    addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
>>>>>>>   optimizedSelectors := Set new!
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
>


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Compiler-eem.380.mcz

Tobias Pape
In reply to this post by Nicolas Cellier
Hi Nicolas,

> On 23.03.2018, at 18:23, Nicolas Cellier <[hidden email]> wrote:
>
> Hi Tobias,
> Yes, some months ago I changed Undeclared to automagically clean-up when bindings are no more used.
> This was necessary to workaround some Environment bugs, and it is a nice feature anyway.
> So, if you want some Undeclared binding to persist, then you must have a strong pointer on it.
> IOW, there should be some class with source code refering to this undeclared binding...
> From what I see, there is such reference in http://source.squeak.org/trunk/Compiler-eem.379.diff
> If there is a reference to this version in the update map, then update should work (and it did for me).

In the end, here too...

>
> 2018-03-23 16:53 GMT+01:00 Eliot Miranda <[hidden email]>:
> Hi Tobias,
>
> On Mar 23, 2018, at 7:56 AM, Tobias Pape <[hidden email]> wrote:
>
>> Heho
>>
>>> On 23.03.2018, at 15:26, Eliot Miranda <[hidden email]> wrote:
>>>
>>> Hi Tobias,
>>>
>>>> On Mar 23, 2018, at 7:03 AM, Tobias Pape <[hidden email]> wrote:
>>>>
>>>> HI Eliot,
>>>>
>>>>> On 23.03.2018, at 14:55, Eliot Miranda <[hidden email]> wrote:
>>>>>
>>>>> Hi Tobias,
>>>>>
>>>>> as the commit comments says, you must first load Compiler-eem.379.  The update steam contains an update for it.  And its commit comment explains the issue:
>>>>
>>>> Yes, I have 379 loaded,
>>>> no it does not help.
>>>>
>>>> Reason: The class is reshaped before its methods are recompiled, hence 'addedSelectorAndMethodClassLiterals' is gone and nilled and throws a MustBeBoolean upon next use, wich is any kind of compilation, so the recompilation fails.
>>>
>>> That's strange.  I tested this multiple times (in 64-bit images) before I committed (as two file-ins), and after I committed (as an update) and saw no problems.  379 adds versions that update both variables, so addedExtraLiterals is in Undeclared and both  it and the addedSelectorAndMethodClassLiterals isn't var get assigned on encoder initialization.  Then, when Encoder gets reshaped, addedSelectorAndMethodClassLiterals is out in Undeclared, addedExtraLiterals becomes the inst var, and both get initialized.  Finally, the references to addedSelectorAndMethodClassLiterals are replaced by references to  addedExtraLiterals, and the putch is complete.
>>>
>>> So you should find addedExtraLiterals in Undeclared, with the value true before loading 380.  
>>
>> Yes:
>> ===
>> Undeclared "a WeakIdentityDictionary(#addedExtraLiterals->false )"
>> (Encoder >> #allLiterals) "first bytecode is pushRcvr: 14, aka addedSelectorAndMethodClassLiterals)
>> ===
>> and " addedSelectorAndMethodClassLiterals := addedExtraLiterals := false." is present.
>>
>> The patch also looks good (especially , change allLiterals before the class reshape)
>> <Bildschirmfoto 2018-03-23 um 15.44.47.PNG>
>>
>>
>> Yet, MCPackageLoader>>basicLoad changes that and pulls everything except the method defs to the front
>> ""Pass 1: Load everything but the methods,  which are collected in methodAdditions.""
>>
>>
>>> Then, after the reshape, but before loading the other defs, you should find addedSelectorAndMethodClassLiterals with the value true.
>>
>> And, yes, After the step 1, undeclareds are as you expected:
>>
>> Undeclared  "a WeakIdentityDictionary(#addedExtraLiterals->false #addedSelectorAndMethodClassLiterals->true )"
>>
>> And Encoder>>allLiterals has the expected literal for the undeclared var.
>>
>> =-=-=-==
>> And now I was able to step though everything just fine (after it DIDN'T work four times this morning), I feel a bit like a moron -.-'
>>
>> Sorry Eliot, everything seems fine *sigh*
>>
>>
>> CARRY ON FOLKS NOTHNG TO SEE KEEP WALKING …
>
> Hang on !! :-). Stepping through isn't a solution.  If you use, for example, Update Squeak from the Squeak menu does it work or not?  I'd still like to understand d why it broke for you...
>
>>
>> best regards
>> -tobias
>>
>>
>>>
>>> The first thing to determine before we throw away 380 and try again is if anyone else is being bitten by this.  If it works for everyone else (and as I said it has worked for me) then there's something broken in your image.  If it's broken for everyone then I guess the package prologue may need to add addedSelectorAndMethodClassLiterals to Undeclared with value false before the reshape and redefinition. You could test if this would work by manually adding addedSelectorAndMethodClassLiterals to Undeclared with value false before loading 380.
>>>
>>> So has anyone else had their system broken by Compiler-eem.380?
>>>
>>> (And I'm really sorry Tobias; I thought I had this one right; and Bert, I think this is an example where update naps are absolutely required so it might be worth adding to the update doc; I'll take a look).
>>>
>>>>
>>>> Best regards
>>>>   -Tobias
>>>>
>>>>
>>>>
>>>>>
>>>>> Name: Compiler-eem.379
>>>>> Author: eem
>>>>> Time: 20 March 2018, 3:27:27.12646 pm
>>>>> UUID: b3856f24-9d98-478a-936f-c6d24d667be4
>>>>> Ancestors: Compiler-eem.378
>>>>>
>>>>> Add initialization of the Undeclared variable addedExtraLiterals which is soon to be a replacement for Encoder's addedSelectorAndMethodClassLiterals, a name which is now misleading given the new full block support.  By adding the initialization of the Undeclared variable the compiler is not broken as the instance variable is renamed and Encoder's methods are recompiled.
>>>>>
>>>>> _,,,^..^,,,_ (phone)
>>>>>
>>>>>> On Mar 23, 2018, at 1:20 AM, Tobias Pape <[hidden email]> wrote:
>>>>>>
>>>>>> Hi Eliot,
>>>>>>
>>>>>>
>>>>>> loading .380  produces an error for me in #allLiterals.
>>>>>>
>>>>>> I think this is because of the Encoder being used for changing the encoder?
>>>>>> we need a few thing here probably:
>>>>>> 1. remove .380
>>>>>> 2. a commit that _Adds_ the new inst var (iirc this is .379 already)
>>>>>> 3. an mcm for that (iirc this is .431.mcm)
>>>>>> 4. remove config .432.mcm
>>>>>> 5. a commit that _uses_ the new inst var
>>>>>> 6. an mcm for that (a _new_ .432.mcm)
>>>>>> 7. a commit that _Removes_ the old inst var.
>>>>>> (8. maybe an mcm for that..)
>>>>>>
>>>>>> Best regards
>>>>>>   -Tobias
>>>>>>
>>>>>>
>>>>>>
>>>>>> <Bildschirmfoto 2018-03-23 um 09.02.37.PNG>
>>>>>>> On 20.03.2018, at 23:30, [hidden email] wrote:
>>>>>>>
>>>>>>> Eliot Miranda uploaded a new version of Compiler to project The Trunk:
>>>>>>> http://source.squeak.org/trunk/Compiler-eem.380.mcz
>>>>>>>
>>>>>>> ==================== Summary ====================
>>>>>>>
>>>>>>> Name: Compiler-eem.380
>>>>>>> Author: eem
>>>>>>> Time: 20 March 2018, 3:30:10.256928 pm
>>>>>>> UUID: 3133d60f-54b2-410e-92ae-ef5cc782ab9c
>>>>>>> Ancestors: Compiler-eem.379
>>>>>>>
>>>>>>> Rename Encoder's addedSelectorAndMethodClassLiterals, a name which is now misleading given the new full block support, to addedExtraLiterals.  Requires Compiler-eem.379.
>>>>>>>
>>>>>>> =============== Diff against Compiler-eem.379 ===============
>>>>>>>
>>>>>>> Item was changed:
>>>>>>> ----- Method: BytecodeEncoder>>allLiteralsForBlockMethod (in category 'results') -----
>>>>>>> allLiteralsForBlockMethod
>>>>>>> +    addedExtraLiterals ifFalse:
>>>>>>> +        [addedExtraLiterals := true.
>>>>>>> -    "N.B. addedSelectorAndMethodClassLiterals is a misnomer.  It should be something like
>>>>>>> -     addedImplicitLiterals or addedExtraLiterals."
>>>>>>> -    addedSelectorAndMethodClassLiterals ifFalse:
>>>>>>> -        [addedSelectorAndMethodClassLiterals := true.
>>>>>>>       "Put the optimized selectors in literals so as to browse senders more easily"
>>>>>>>       optimizedSelectors := optimizedSelectors reject: [:e| literalStream originalContents hasLiteral: e].
>>>>>>>       optimizedSelectors isEmpty ifFalse: [
>>>>>>>           "Use one entry per literal if enough room, else make anArray"
>>>>>>>           literalStream position + optimizedSelectors size + 2 >= self maxNumLiterals
>>>>>>>               ifTrue: [self litIndex: optimizedSelectors asArray]
>>>>>>>               ifFalse: [optimizedSelectors do: [:e | self litIndex: e]]].
>>>>>>>       "Add a slot for outerCode"
>>>>>>>       self litIndex: nil].
>>>>>>>   ^literalStream contents!
>>>>>>>
>>>>>>> Item was changed:
>>>>>>> ----- Method: BytecodeEncoder>>resetForFullBlockGeneration (in category 'code generation') -----
>>>>>>> resetForFullBlockGeneration
>>>>>>>   literalStream := WriteStream on: (Array new: 8).
>>>>>>> +    addedExtraLiterals := false.
>>>>>>> -    addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
>>>>>>>   optimizedSelectors := Set new!
>>>>>>>
>>>>>>> Item was changed:
>>>>>>> ----- Method: BytecodeEncoder>>resetLiteralStreamForFullBlock (in category 'code generation') -----
>>>>>>> resetLiteralStreamForFullBlock
>>>>>>>   literalStream := WriteStream on: (Array new: 32).
>>>>>>> +    addedExtraLiterals := false.
>>>>>>> -    addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
>>>>>>>   optimizedSelectors := Set new!
>>>>>>>
>>>>>>> Item was changed:
>>>>>>> ParseNode subclass: #Encoder
>>>>>>> +    instanceVariableNames: 'scopeTable nTemps supered requestor class selector literalStream selectorSet litIndSet litSet sourceRanges globalSourceRanges addedExtraLiterals optimizedSelectors cue'
>>>>>>> -    instanceVariableNames: 'scopeTable nTemps supered requestor class selector literalStream selectorSet litIndSet litSet sourceRanges globalSourceRanges addedSelectorAndMethodClassLiterals optimizedSelectors cue'
>>>>>>>   classVariableNames: ''
>>>>>>>   poolDictionaries: ''
>>>>>>>   category: 'Compiler-Kernel'!
>>>>>>>
>>>>>>> !Encoder commentStamp: 'cwp 12/26/2012 23:29' prior: 0!
>>>>>>> I encode names and literals into tree nodes with byte codes for the compiler. Byte codes for literals are not assigned until the tree-sizing pass of the compiler, because only then is it known which literals are actually needed. I also keep track of sourceCode ranges during parsing and code generation so I can provide an inverse map for the debugger.!
>>>>>>>
>>>>>>> Item was changed:
>>>>>>> ----- Method: Encoder>>allLiterals (in category 'results') -----
>>>>>>> allLiterals
>>>>>>> +    addedExtraLiterals ifFalse:
>>>>>>> +        [addedExtraLiterals := true.
>>>>>>> -    addedSelectorAndMethodClassLiterals ifFalse:
>>>>>>> -        [addedSelectorAndMethodClassLiterals := true.
>>>>>>>       "Put the optimized selectors in literals so as to browse senders more easily"
>>>>>>>       optimizedSelectors := optimizedSelectors reject: [:e| literalStream originalContents hasLiteral: e].
>>>>>>>       optimizedSelectors isEmpty ifFalse: [
>>>>>>>           "Use one entry per literal if enough room, else make anArray"
>>>>>>>           literalStream position + optimizedSelectors size + 2 >= self maxNumLiterals
>>>>>>>               ifTrue: [self litIndex: optimizedSelectors asArray]
>>>>>>>               ifFalse: [optimizedSelectors do: [:e | self litIndex: e]]].
>>>>>>>       "Add a slot for selector or MethodProperties"
>>>>>>>       self litIndex: nil.
>>>>>>>       self litIndex: self associationForClass].
>>>>>>>   ^literalStream contents!
>>>>>>>
>>>>>>> Item was changed:
>>>>>>> ----- Method: Encoder>>initScopeAndLiteralTables (in category 'initialize-release') -----
>>>>>>> initScopeAndLiteralTables
>>>>>>>
>>>>>>>   scopeTable := StdVariables copy.
>>>>>>>   litSet := StdLiterals copy.
>>>>>>>   "comments can be left hanging on nodes from previous compilations.
>>>>>>>    probably better than this hack fix is to create the nodes afresh on each compilation."
>>>>>>>   scopeTable do:
>>>>>>>       [:varNode| varNode comment: nil].
>>>>>>>   litSet do:
>>>>>>>       [:varNode| varNode comment: nil].
>>>>>>>   selectorSet := StdSelectors copy.
>>>>>>>   litIndSet := Dictionary new: 16.
>>>>>>>   literalStream := WriteStream on: (Array new: 32).
>>>>>>> +    addedExtraLiterals := false.
>>>>>>> -    addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
>>>>>>>   optimizedSelectors := Set new!
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
>
>
>
>
>