The Trunk: Compiler-eem.387.mcz

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

The Trunk: Compiler-eem.387.mcz

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

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

Name: Compiler-eem.387
Author: eem
Time: 16 June 2018, 6:15:08.755516 pm
UUID: 2a8c2667-3328-4cd4-a7c9-c98e04d6ddf2
Ancestors: Compiler-eem.386

Fix the decompiler for the full lock regime.  There isn't much in the decompiler that directly refers to the method inst var (most accesses use self method, which answers the sender inst var of InstructionStream).  But case statements is one of them (specifically myExits := myExits reject: [ :e | e isNil or: [ e < 0 or: [ e > method endPC ] ] ]).  The bug was that doClosureCopy:copiedValues: was only setting up sender but not method and so case statements were mishandled within full blocks (e.g. in LanguageEditor>>asHtml:) becausde the wrong endPC was being used.

=============== Diff against Compiler-eem.386 ===============

Item was changed:
  ----- Method: Decompiler>>doClosureCopy:copiedValues: (in category 'control') -----
  doClosureCopy: aCompiledBlock copiedValues: blockCopiedValues
  | savedTemps savedTempVarCount savedNumLocalTemps savedMethod savedPC
   blockArgs blockTemps blockTempsOffset block |
  savedTemps := tempVars.
  savedTempVarCount := tempVarCount.
  savedNumLocalTemps := numLocalTemps.
  numLocalTemps := aCompiledBlock numTemps - aCompiledBlock numArgs - blockCopiedValues size.
  blockTempsOffset := aCompiledBlock numArgs + blockCopiedValues size.
  (blockStartsToTempVars notNil "implies we were intialized with temp names."
  and: [blockStartsToTempVars includesKey: aCompiledBlock])
  ifTrue:
  [tempVars := blockStartsToTempVars at: aCompiledBlock]
  ifFalse:
  [blockArgs := (1 to: aCompiledBlock numArgs) collect:
  [:i| (constructor
  codeTemp: i - 1
  named: 't', (tempVarCount + i) printString)
   beBlockArg].
  blockTemps := (1 to: numLocalTemps) collect:
  [:i| constructor
  codeTemp: i + blockTempsOffset - 1
  named: 't', (tempVarCount + i + aCompiledBlock numArgs) printString].
  tempVars := blockArgs, blockCopiedValues, blockTemps].
  tempVarCount := tempVarCount + aCompiledBlock numArgs + numLocalTemps.
  savedMethod := self method. savedPC := pc.
+ super method: (method := aCompiledBlock) pc: aCompiledBlock initialPC.
- super method: aCompiledBlock pc: aCompiledBlock initialPC.
  block := [self blockTo: aCompiledBlock endPC]
+ ensure: [super method: (method := savedMethod) pc: savedPC].
- ensure: [super method: savedMethod pc: savedPC].
  stack addLast: ((constructor
  codeArguments: (tempVars copyFrom: 1 to: aCompiledBlock numArgs)
  temps: (tempVars copyFrom: blockTempsOffset + 1 to: blockTempsOffset + numLocalTemps)
  block: block)
  pc: aCompiledBlock -> pc; "c.f. BytecodeEncoder>>pc"
  yourself).
  tempVars := savedTemps.
  tempVarCount := savedTempVarCount.
  numLocalTemps := savedNumLocalTemps!


Reply | Threaded
Open this post in threaded view
|

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

David T. Lewis
I really love to see good documentation and good commit notices. This one
is a sterling example. It is brief and clear. It says what was changed, why
it was changed, and provides a small code snippet because that is needed to
understand reason for the change.

Someone will be able to read this commit notice 10 or 20 years from now, and
understand exactly why those two lines of code were changed.

Dave


On Sun, Jun 17, 2018 at 01:15:23AM +0000, [hidden email] wrote:

> Eliot Miranda uploaded a new version of Compiler to project The Trunk:
> http://source.squeak.org/trunk/Compiler-eem.387.mcz
>
> ==================== Summary ====================
>
> Name: Compiler-eem.387
> Author: eem
> Time: 16 June 2018, 6:15:08.755516 pm
> UUID: 2a8c2667-3328-4cd4-a7c9-c98e04d6ddf2
> Ancestors: Compiler-eem.386
>
> Fix the decompiler for the full lock regime.  There isn't much in the decompiler that directly refers to the method inst var (most accesses use self method, which answers the sender inst var of InstructionStream).  But case statements is one of them (specifically myExits := myExits reject: [ :e | e isNil or: [ e < 0 or: [ e > method endPC ] ] ]).  The bug was that doClosureCopy:copiedValues: was only setting up sender but not method and so case statements were mishandled within full blocks (e.g. in LanguageEditor>>asHtml:) becausde the wrong endPC was being used.
>
> =============== Diff against Compiler-eem.386 ===============
>
> Item was changed:
>   ----- Method: Decompiler>>doClosureCopy:copiedValues: (in category 'control') -----
>   doClosureCopy: aCompiledBlock copiedValues: blockCopiedValues
>   | savedTemps savedTempVarCount savedNumLocalTemps savedMethod savedPC
>    blockArgs blockTemps blockTempsOffset block |
>   savedTemps := tempVars.
>   savedTempVarCount := tempVarCount.
>   savedNumLocalTemps := numLocalTemps.
>   numLocalTemps := aCompiledBlock numTemps - aCompiledBlock numArgs - blockCopiedValues size.
>   blockTempsOffset := aCompiledBlock numArgs + blockCopiedValues size.
>   (blockStartsToTempVars notNil "implies we were intialized with temp names."
>   and: [blockStartsToTempVars includesKey: aCompiledBlock])
>   ifTrue:
>   [tempVars := blockStartsToTempVars at: aCompiledBlock]
>   ifFalse:
>   [blockArgs := (1 to: aCompiledBlock numArgs) collect:
>   [:i| (constructor
>   codeTemp: i - 1
>   named: 't', (tempVarCount + i) printString)
>    beBlockArg].
>   blockTemps := (1 to: numLocalTemps) collect:
>   [:i| constructor
>   codeTemp: i + blockTempsOffset - 1
>   named: 't', (tempVarCount + i + aCompiledBlock numArgs) printString].
>   tempVars := blockArgs, blockCopiedValues, blockTemps].
>   tempVarCount := tempVarCount + aCompiledBlock numArgs + numLocalTemps.
>   savedMethod := self method. savedPC := pc.
> + super method: (method := aCompiledBlock) pc: aCompiledBlock initialPC.
> - super method: aCompiledBlock pc: aCompiledBlock initialPC.
>   block := [self blockTo: aCompiledBlock endPC]
> + ensure: [super method: (method := savedMethod) pc: savedPC].
> - ensure: [super method: savedMethod pc: savedPC].
>   stack addLast: ((constructor
>   codeArguments: (tempVars copyFrom: 1 to: aCompiledBlock numArgs)
>   temps: (tempVars copyFrom: blockTempsOffset + 1 to: blockTempsOffset + numLocalTemps)
>   block: block)
>   pc: aCompiledBlock -> pc; "c.f. BytecodeEncoder>>pc"
>   yourself).
>   tempVars := savedTemps.
>   tempVarCount := savedTempVarCount.
>   numLocalTemps := savedNumLocalTemps!
>
>