Nicolas Cellier uploaded a new version of Compiler to project The Trunk:
http://source.squeak.org/trunk/Compiler-nice.129.mcz ==================== Summary ==================== Name: Compiler-nice.129 Author: nice Time: 1 March 2010, 11:24:30.271 pm UUID: a27b827d-1354-1147-bc1a-edeb6e1f0e97 Ancestors: Compiler-nice.128 Fix Compiler/Decompiler temporary slot mismatch http://bugs.squeak.org/view.php?id=7467 See also my comments in response to http://lists.squeakfoundation.org/pipermail/squeak-dev/2010-March/145280.html Eliot, please check.... =============== Diff against Compiler-nice.128 =============== Item was changed: InstructionStream subclass: #Decompiler instanceVariableNames: 'constructor method instVars tempVars constTable stack statements lastPc exit caseExits lastJumpPc lastReturnPc limit hasValue blockStackBase numLocalTemps blockStartsToTempVars tempVarCount' classVariableNames: 'ArgumentFlag CascadeFlag CaseFlag IfNilFlag' poolDictionaries: '' category: 'Compiler-Kernel'! + !Decompiler commentStamp: 'nice 3/1/2010 19:56' prior: 0! - !Decompiler commentStamp: '<historical>' prior: 0! I decompile a method in three phases: Reverser: postfix byte codes -> prefix symbolic codes (nodes and atoms) Parser: prefix symbolic codes -> node tree (same as the compiler) Printer: node tree -> text (done by the nodes) instance vars: + constructor <DecompilerConstructor> an auxiliary knowing how to generate Abstract Syntax Tree (node tree) + method <CompiledMethod> the method being decompiled + instVars <Array of: String> the instance variables of the class implementing method + tempVars <String | (OrderedCollection of: String)> hold the names of temporary variables (if known) + NOTE: POLYMORPHISM WILL BE RESOLVED IN #initSymbols: + constTable <Collection of: ParseNode> parse node associated with byte encoded constants (nil true false 0 1 -1 etc...) + stack <OrderedCollection of: (ParseNode | String | Integer) > multipurpose... + statements <OrderedCollection of: ParseNode> the statements of the method being decompiled + lastPc <Integer> + exit <Integer> + caseExits <OrderedCollection of: Integer> - stack of exit addresses that have been seen in the branches of caseOf:'s + lastJumpPc <Integer> + lastReturnPc <Integer> + limit <Integer> + hasValue <Boolean> + blockStackBase <Integer> + numLocaltemps <Integer | Symbol> - number of temps local to a block; also a flag indicating decompiling a block + blockStartsToTempVars <Dictionary key: Integer value: (OrderedCollection of: String)> + tempVarCount <Integer> number of temp vars used by the method! - constructor - method - instVars - tempVars - constTable - stack - statements - lastPc - exit - caseExits - stack of exit addresses that have been seen in the branches of caseOf:'s - lastJumpPc - lastReturnPc - limit - hasValue - blockStackBase - numLocaltemps - number of temps local to a block; also a flag indicating decompiling a block! Item was added: + ----- Method: BlockNode>>makeTemporariesRemovable (in category 'code generation (closures)') ----- + makeTemporariesRemovable + "Utilities for when we want to remove some temporaries." + + temporaries isArray ifTrue: + [temporaries := temporaries asOrderedCollection].! Item was changed: ----- Method: BlockNode>>addRemoteTemp:rootNode: (in category 'code generation (closures)') ----- addRemoteTemp: aTempVariableNode rootNode: rootNode "<MethodNode>" "Add aTempVariableNode to my actualScope's sequence of remote temps. If I am an optimized block then the actual scope is my actualScopeIfOptimized, otherwise it is myself." - temporaries isArray ifTrue: - [temporaries := temporaries asOrderedCollection]. remoteTempNode == nil ifTrue: [remoteTempNode := RemoteTempVectorNode new name: self remoteTempNodeName index: arguments size + temporaries size type: LdTempType scope: 0. actualScopeIfOptimized ifNil: + [self addTempNode: remoteTempNode. - [temporaries addLast: remoteTempNode. remoteTempNode definingScope: self] ifNotNil: [actualScopeIfOptimized addHoistedTemps: { remoteTempNode }]]. remoteTempNode addRemoteTemp: aTempVariableNode encoder: rootNode encoder. "use remove:ifAbsent: because the deferred analysis for optimized loops can result in the temp has already been hoised into the root." + self actualScope removeTempNode: aTempVariableNode ifAbsent: []. - temporaries remove: aTempVariableNode ifAbsent: []. ^remoteTempNode! Item was added: + ----- Method: BlockNode>>addTempNode: (in category 'code generation (closures)') ----- + addTempNode: aTempVariableNode + "Utilities for when we want to add some temporaries." + + self makeTemporariesRemovable. + ^temporaries add: aTempVariableNode! Item was added: + ----- Method: BlockNode>>removeTempNode:ifAbsent: (in category 'code generation (closures)') ----- + removeTempNode: aTempVariableNode ifAbsent: aBlock + "Utilities for when we want to remove some temporaries." + + self makeTemporariesRemovable. + ^temporaries remove: aTempVariableNode ifAbsent: aBlock + ! |
Thanks for the fix Nicolas.
Too bad #asOrderedCollection creates a copy even if it's sent to an OrderedCollection. I wonder if there's a way to fix it without breaking existing code. Levente On Mon, 1 Mar 2010, [hidden email] wrote: > Nicolas Cellier uploaded a new version of Compiler to project The Trunk: > http://source.squeak.org/trunk/Compiler-nice.129.mcz > > ==================== Summary ==================== > > Name: Compiler-nice.129 > Author: nice > Time: 1 March 2010, 11:24:30.271 pm > UUID: a27b827d-1354-1147-bc1a-edeb6e1f0e97 > Ancestors: Compiler-nice.128 > > Fix Compiler/Decompiler temporary slot mismatch > http://bugs.squeak.org/view.php?id=7467 > > See also my comments in response to > http://lists.squeakfoundation.org/pipermail/squeak-dev/2010-March/145280.html > > Eliot, please check.... > > =============== Diff against Compiler-nice.128 =============== > > Item was changed: > InstructionStream subclass: #Decompiler > instanceVariableNames: 'constructor method instVars tempVars constTable stack statements lastPc exit caseExits lastJumpPc lastReturnPc limit hasValue blockStackBase numLocalTemps blockStartsToTempVars tempVarCount' > classVariableNames: 'ArgumentFlag CascadeFlag CaseFlag IfNilFlag' > poolDictionaries: '' > category: 'Compiler-Kernel'! > > + !Decompiler commentStamp: 'nice 3/1/2010 19:56' prior: 0! > - !Decompiler commentStamp: '<historical>' prior: 0! > I decompile a method in three phases: > Reverser: postfix byte codes -> prefix symbolic codes (nodes and atoms) > Parser: prefix symbolic codes -> node tree (same as the compiler) > Printer: node tree -> text (done by the nodes) > > > instance vars: > > + constructor <DecompilerConstructor> an auxiliary knowing how to generate Abstract Syntax Tree (node tree) > + method <CompiledMethod> the method being decompiled > + instVars <Array of: String> the instance variables of the class implementing method > + tempVars <String | (OrderedCollection of: String)> hold the names of temporary variables (if known) > + NOTE: POLYMORPHISM WILL BE RESOLVED IN #initSymbols: > + constTable <Collection of: ParseNode> parse node associated with byte encoded constants (nil true false 0 1 -1 etc...) > + stack <OrderedCollection of: (ParseNode | String | Integer) > multipurpose... > + statements <OrderedCollection of: ParseNode> the statements of the method being decompiled > + lastPc <Integer> > + exit <Integer> > + caseExits <OrderedCollection of: Integer> - stack of exit addresses that have been seen in the branches of caseOf:'s > + lastJumpPc <Integer> > + lastReturnPc <Integer> > + limit <Integer> > + hasValue <Boolean> > + blockStackBase <Integer> > + numLocaltemps <Integer | Symbol> - number of temps local to a block; also a flag indicating decompiling a block > + blockStartsToTempVars <Dictionary key: Integer value: (OrderedCollection of: String)> > + tempVarCount <Integer> number of temp vars used by the method! > - constructor > - method > - instVars > - tempVars > - constTable > - stack > - statements > - lastPc > - exit > - caseExits - stack of exit addresses that have been seen in the branches of caseOf:'s > - lastJumpPc > - lastReturnPc > - limit > - hasValue > - blockStackBase > - numLocaltemps - number of temps local to a block; also a flag indicating decompiling a block! > > Item was added: > + ----- Method: BlockNode>>makeTemporariesRemovable (in category 'code generation (closures)') ----- > + makeTemporariesRemovable > + "Utilities for when we want to remove some temporaries." > + > + temporaries isArray ifTrue: > + [temporaries := temporaries asOrderedCollection].! > > Item was changed: > ----- Method: BlockNode>>addRemoteTemp:rootNode: (in category 'code generation (closures)') ----- > addRemoteTemp: aTempVariableNode rootNode: rootNode "<MethodNode>" > "Add aTempVariableNode to my actualScope's sequence of > remote temps. If I am an optimized block then the actual > scope is my actualScopeIfOptimized, otherwise it is myself." > - temporaries isArray ifTrue: > - [temporaries := temporaries asOrderedCollection]. > remoteTempNode == nil ifTrue: > [remoteTempNode := RemoteTempVectorNode new > name: self remoteTempNodeName > index: arguments size + temporaries size > type: LdTempType > scope: 0. > actualScopeIfOptimized > ifNil: > + [self addTempNode: remoteTempNode. > - [temporaries addLast: remoteTempNode. > remoteTempNode definingScope: self] > ifNotNil: [actualScopeIfOptimized addHoistedTemps: { remoteTempNode }]]. > remoteTempNode addRemoteTemp: aTempVariableNode encoder: rootNode encoder. > "use remove:ifAbsent: because the deferred analysis for optimized > loops can result in the temp has already been hoised into the root." > + self actualScope removeTempNode: aTempVariableNode ifAbsent: []. > - temporaries remove: aTempVariableNode ifAbsent: []. > ^remoteTempNode! > > Item was added: > + ----- Method: BlockNode>>addTempNode: (in category 'code generation (closures)') ----- > + addTempNode: aTempVariableNode > + "Utilities for when we want to add some temporaries." > + > + self makeTemporariesRemovable. > + ^temporaries add: aTempVariableNode! > > Item was added: > + ----- Method: BlockNode>>removeTempNode:ifAbsent: (in category 'code generation (closures)') ----- > + removeTempNode: aTempVariableNode ifAbsent: aBlock > + "Utilities for when we want to remove some temporaries." > + > + self makeTemporariesRemovable. > + ^temporaries remove: aTempVariableNode ifAbsent: aBlock > + ! > > > |
I don't mind optimization there, asOrderedCollection was already in
Eliot's code... What worries me is that this is not a complete fix... For example, try (EventSensor>>#eventTickler) decompileString. or try your own example: Compiler evaluate: ' [ | foo | self halt. [ foo := false ] value ] whileTrue' It has to do with the definingScope not being the current readingScopes nor the actualScope... I fear more bugs in case of shadowed temps (not possible interactively though, Compiler is pedantic). Nicolas 2010/3/2 Levente Uzonyi <[hidden email]>: > Thanks for the fix Nicolas. > Too bad #asOrderedCollection creates a copy even if it's sent to an > OrderedCollection. I wonder if there's a way to fix it without breaking > existing code. > > > Levente > > On Mon, 1 Mar 2010, [hidden email] wrote: > >> Nicolas Cellier uploaded a new version of Compiler to project The Trunk: >> http://source.squeak.org/trunk/Compiler-nice.129.mcz >> >> ==================== Summary ==================== >> >> Name: Compiler-nice.129 >> Author: nice >> Time: 1 March 2010, 11:24:30.271 pm >> UUID: a27b827d-1354-1147-bc1a-edeb6e1f0e97 >> Ancestors: Compiler-nice.128 >> >> Fix Compiler/Decompiler temporary slot mismatch >> http://bugs.squeak.org/view.php?id=7467 >> >> See also my comments in response to >> >> http://lists.squeakfoundation.org/pipermail/squeak-dev/2010-March/145280.html >> >> Eliot, please check.... >> >> =============== Diff against Compiler-nice.128 =============== >> >> Item was changed: >> InstructionStream subclass: #Decompiler >> instanceVariableNames: 'constructor method instVars tempVars >> constTable stack statements lastPc exit caseExits lastJumpPc lastReturnPc >> limit hasValue blockStackBase numLocalTemps blockStartsToTempVars >> tempVarCount' >> classVariableNames: 'ArgumentFlag CascadeFlag CaseFlag IfNilFlag' >> poolDictionaries: '' >> category: 'Compiler-Kernel'! >> >> + !Decompiler commentStamp: 'nice 3/1/2010 19:56' prior: 0! >> - !Decompiler commentStamp: '<historical>' prior: 0! >> I decompile a method in three phases: >> Reverser: postfix byte codes -> prefix symbolic codes (nodes and >> atoms) >> Parser: prefix symbolic codes -> node tree (same as the compiler) >> Printer: node tree -> text (done by the nodes) >> >> >> instance vars: >> >> + constructor <DecompilerConstructor> an auxiliary knowing how to >> generate Abstract Syntax Tree (node tree) >> + method <CompiledMethod> the method being decompiled >> + instVars <Array of: String> the instance variables of the class >> implementing method >> + tempVars <String | (OrderedCollection of: String)> hold the names >> of temporary variables (if known) >> + NOTE: POLYMORPHISM WILL BE RESOLVED IN #initSymbols: >> + constTable <Collection of: ParseNode> parse node associated with >> byte encoded constants (nil true false 0 1 -1 etc...) >> + stack <OrderedCollection of: (ParseNode | String | Integer) > >> multipurpose... >> + statements <OrderedCollection of: ParseNode> the statements of the >> method being decompiled >> + lastPc <Integer> >> + exit <Integer> >> + caseExits <OrderedCollection of: Integer> - stack of exit >> addresses that have been seen in the branches of caseOf:'s >> + lastJumpPc <Integer> >> + lastReturnPc <Integer> >> + limit <Integer> >> + hasValue <Boolean> >> + blockStackBase <Integer> >> + numLocaltemps <Integer | Symbol> - number of temps local to a >> block; also a flag indicating decompiling a block >> + blockStartsToTempVars <Dictionary key: Integer value: >> (OrderedCollection of: String)> >> + tempVarCount <Integer> number of temp vars used by the method! >> - constructor >> - method >> - instVars >> - tempVars >> - constTable >> - stack >> - statements >> - lastPc >> - exit >> - caseExits - stack of exit addresses that have been seen in >> the branches of caseOf:'s >> - lastJumpPc >> - lastReturnPc >> - limit >> - hasValue >> - blockStackBase >> - numLocaltemps - number of temps local to a block; also a flag >> indicating decompiling a block! >> >> Item was added: >> + ----- Method: BlockNode>>makeTemporariesRemovable (in category 'code >> generation (closures)') ----- >> + makeTemporariesRemovable >> + "Utilities for when we want to remove some temporaries." >> + >> + temporaries isArray ifTrue: >> + [temporaries := temporaries asOrderedCollection].! >> >> Item was changed: >> ----- Method: BlockNode>>addRemoteTemp:rootNode: (in category 'code >> generation (closures)') ----- >> addRemoteTemp: aTempVariableNode rootNode: rootNode "<MethodNode>" >> "Add aTempVariableNode to my actualScope's sequence of >> remote temps. If I am an optimized block then the actual >> scope is my actualScopeIfOptimized, otherwise it is myself." >> - temporaries isArray ifTrue: >> - [temporaries := temporaries asOrderedCollection]. >> remoteTempNode == nil ifTrue: >> [remoteTempNode := RemoteTempVectorNode new >> name: self >> remoteTempNodeName >> index: >> arguments size + temporaries size >> type: >> LdTempType >> scope: 0. >> actualScopeIfOptimized >> ifNil: >> + [self addTempNode: remoteTempNode. >> - [temporaries addLast: remoteTempNode. >> remoteTempNode definingScope: self] >> ifNotNil: [actualScopeIfOptimized addHoistedTemps: >> { remoteTempNode }]]. >> remoteTempNode addRemoteTemp: aTempVariableNode encoder: rootNode >> encoder. >> "use remove:ifAbsent: because the deferred analysis for optimized >> loops can result in the temp has already been hoised into the >> root." >> + self actualScope removeTempNode: aTempVariableNode ifAbsent: []. >> - temporaries remove: aTempVariableNode ifAbsent: []. >> ^remoteTempNode! >> >> Item was added: >> + ----- Method: BlockNode>>addTempNode: (in category 'code generation >> (closures)') ----- >> + addTempNode: aTempVariableNode >> + "Utilities for when we want to add some temporaries." >> + >> + self makeTemporariesRemovable. >> + ^temporaries add: aTempVariableNode! >> >> Item was added: >> + ----- Method: BlockNode>>removeTempNode:ifAbsent: (in category 'code >> generation (closures)') ----- >> + removeTempNode: aTempVariableNode ifAbsent: aBlock >> + "Utilities for when we want to remove some temporaries." >> + >> + self makeTemporariesRemovable. >> + ^temporaries remove: aTempVariableNode ifAbsent: aBlock >> + ! >> >> >> > > |
On Tue, 2 Mar 2010, Nicolas Cellier wrote:
> I don't mind optimization there, asOrderedCollection was already in > Eliot's code... Well, you kept Eliot's optimization (#isArray check) instead of just sending #asOrderedCollection. > What worries me is that this is not a complete fix... > > For example, try > > (EventSensor>>#eventTickler) decompileString. > > or try your own example: > > Compiler evaluate: ' [ > | foo | > self halt. > [ foo := false ] value ] whileTrue' > they seem to be closely related to each other. Levente > It has to do with the definingScope not being the current > readingScopes nor the actualScope... > I fear more bugs in case of shadowed temps (not possible interactively > though, Compiler is pedantic). > > Nicolas > > 2010/3/2 Levente Uzonyi <[hidden email]>: >> Thanks for the fix Nicolas. >> Too bad #asOrderedCollection creates a copy even if it's sent to an >> OrderedCollection. I wonder if there's a way to fix it without breaking >> existing code. >> >> >> Levente >> >> On Mon, 1 Mar 2010, [hidden email] wrote: >> >>> Nicolas Cellier uploaded a new version of Compiler to project The Trunk: >>> http://source.squeak.org/trunk/Compiler-nice.129.mcz >>> >>> ==================== Summary ==================== >>> >>> Name: Compiler-nice.129 >>> Author: nice >>> Time: 1 March 2010, 11:24:30.271 pm >>> UUID: a27b827d-1354-1147-bc1a-edeb6e1f0e97 >>> Ancestors: Compiler-nice.128 >>> >>> Fix Compiler/Decompiler temporary slot mismatch >>> http://bugs.squeak.org/view.php?id=7467 >>> >>> See also my comments in response to >>> >>> http://lists.squeakfoundation.org/pipermail/squeak-dev/2010-March/145280.html >>> >>> Eliot, please check.... >>> >>> =============== Diff against Compiler-nice.128 =============== >>> >>> Item was changed: >>> InstructionStream subclass: #Decompiler >>> instanceVariableNames: 'constructor method instVars tempVars >>> constTable stack statements lastPc exit caseExits lastJumpPc lastReturnPc >>> limit hasValue blockStackBase numLocalTemps blockStartsToTempVars >>> tempVarCount' >>> classVariableNames: 'ArgumentFlag CascadeFlag CaseFlag IfNilFlag' >>> poolDictionaries: '' >>> category: 'Compiler-Kernel'! >>> >>> + !Decompiler commentStamp: 'nice 3/1/2010 19:56' prior: 0! >>> - !Decompiler commentStamp: '<historical>' prior: 0! >>> I decompile a method in three phases: >>> Reverser: postfix byte codes -> prefix symbolic codes (nodes and >>> atoms) >>> Parser: prefix symbolic codes -> node tree (same as the compiler) >>> Printer: node tree -> text (done by the nodes) >>> >>> >>> instance vars: >>> >>> + constructor <DecompilerConstructor> an auxiliary knowing how to >>> generate Abstract Syntax Tree (node tree) >>> + method <CompiledMethod> the method being decompiled >>> + instVars <Array of: String> the instance variables of the class >>> implementing method >>> + tempVars <String | (OrderedCollection of: String)> hold the names >>> of temporary variables (if known) >>> + NOTE: POLYMORPHISM WILL BE RESOLVED IN #initSymbols: >>> + constTable <Collection of: ParseNode> parse node associated with >>> byte encoded constants (nil true false 0 1 -1 etc...) >>> + stack <OrderedCollection of: (ParseNode | String | Integer) > >>> multipurpose... >>> + statements <OrderedCollection of: ParseNode> the statements of the >>> method being decompiled >>> + lastPc <Integer> >>> + exit <Integer> >>> + caseExits <OrderedCollection of: Integer> - stack of exit >>> addresses that have been seen in the branches of caseOf:'s >>> + lastJumpPc <Integer> >>> + lastReturnPc <Integer> >>> + limit <Integer> >>> + hasValue <Boolean> >>> + blockStackBase <Integer> >>> + numLocaltemps <Integer | Symbol> - number of temps local to a >>> block; also a flag indicating decompiling a block >>> + blockStartsToTempVars <Dictionary key: Integer value: >>> (OrderedCollection of: String)> >>> + tempVarCount <Integer> number of temp vars used by the method! >>> - constructor >>> - method >>> - instVars >>> - tempVars >>> - constTable >>> - stack >>> - statements >>> - lastPc >>> - exit >>> - caseExits - stack of exit addresses that have been seen in >>> the branches of caseOf:'s >>> - lastJumpPc >>> - lastReturnPc >>> - limit >>> - hasValue >>> - blockStackBase >>> - numLocaltemps - number of temps local to a block; also a flag >>> indicating decompiling a block! >>> >>> Item was added: >>> + ----- Method: BlockNode>>makeTemporariesRemovable (in category 'code >>> generation (closures)') ----- >>> + makeTemporariesRemovable >>> + "Utilities for when we want to remove some temporaries." >>> + >>> + temporaries isArray ifTrue: >>> + [temporaries := temporaries asOrderedCollection].! >>> >>> Item was changed: >>> ----- Method: BlockNode>>addRemoteTemp:rootNode: (in category 'code >>> generation (closures)') ----- >>> addRemoteTemp: aTempVariableNode rootNode: rootNode "<MethodNode>" >>> "Add aTempVariableNode to my actualScope's sequence of >>> remote temps. If I am an optimized block then the actual >>> scope is my actualScopeIfOptimized, otherwise it is myself." >>> - temporaries isArray ifTrue: >>> - [temporaries := temporaries asOrderedCollection]. >>> remoteTempNode == nil ifTrue: >>> [remoteTempNode := RemoteTempVectorNode new >>> name: self >>> remoteTempNodeName >>> index: >>> arguments size + temporaries size >>> type: >>> LdTempType >>> scope: 0. >>> actualScopeIfOptimized >>> ifNil: >>> + [self addTempNode: remoteTempNode. >>> - [temporaries addLast: remoteTempNode. >>> remoteTempNode definingScope: self] >>> ifNotNil: [actualScopeIfOptimized addHoistedTemps: >>> { remoteTempNode }]]. >>> remoteTempNode addRemoteTemp: aTempVariableNode encoder: rootNode >>> encoder. >>> "use remove:ifAbsent: because the deferred analysis for optimized >>> loops can result in the temp has already been hoised into the >>> root." >>> + self actualScope removeTempNode: aTempVariableNode ifAbsent: []. >>> - temporaries remove: aTempVariableNode ifAbsent: []. >>> ^remoteTempNode! >>> >>> Item was added: >>> + ----- Method: BlockNode>>addTempNode: (in category 'code generation >>> (closures)') ----- >>> + addTempNode: aTempVariableNode >>> + "Utilities for when we want to add some temporaries." >>> + >>> + self makeTemporariesRemovable. >>> + ^temporaries add: aTempVariableNode! >>> >>> Item was added: >>> + ----- Method: BlockNode>>removeTempNode:ifAbsent: (in category 'code >>> generation (closures)') ----- >>> + removeTempNode: aTempVariableNode ifAbsent: aBlock >>> + "Utilities for when we want to remove some temporaries." >>> + >>> + self makeTemporariesRemovable. >>> + ^temporaries remove: aTempVariableNode ifAbsent: aBlock >>> + ! >>> >>> >>> >> >> > > |
Free forum by Nabble | Edit this page |