Nicolas Cellier uploaded a new version of Compiler to project The Trunk:
http://source.squeak.org/trunk/Compiler-nice.173.mcz ==================== Summary ==================== Name: Compiler-nice.173 Author: nice Time: 8 September 2010, 1:36:04.973 am UUID: f40f6937-a88a-4f85-b0b0-f68b310a1d92 Ancestors: Compiler-eem.172 Compiler fix for http://bugs.squeak.org/view.php?id=7093 Avoid optimizing a to:do: loop that does modify its limit inside the loop. Thanks Eliot! Note: an alternate solution would be to create a shadow variable, assign it with the limit, and optimize the block. Note 2: the bug report also has a so far uncorrected Decompiler part and cannot be closed yet. =============== Diff against Compiler-eem.172 =============== Item was changed: ----- Method: MessageNode>>transformToDo: (in category 'macro transformations') ----- transformToDo: encoder " var := rcvr. L1: [var <= arg1] Bfp(L2) [block body. var := var + inc] Jmp(L1) L2: " + | limit increment block initStmt test incStmt limitInit blockVar myRange blockRange limitIsAssignedTo | - | limit increment block initStmt test incStmt limitInit blockVar myRange blockRange | "First check for valid arguments" ((arguments last isMemberOf: BlockNode) and: [arguments last numberOfArguments = 1 and: [arguments last firstArgument isVariableReference "As with debugger remote vars"]]) ifFalse: [^false]. arguments size = 3 ifTrue: [increment := arguments at: 2. (increment isConstantNumber + and: [increment literalValue ~= 0]) ifFalse: [^false]] - and: [increment literalValue ~= 0]) ifFalse: [^ false]] ifFalse: [increment := encoder encodeLiteral: 1]. + (limit := arguments at: 1) isVariableReference ifTrue: + [limitIsAssignedTo := false. + arguments last nodesDo: + [:node| + (node isAssignmentNode and: [node variable = limit]) ifTrue: + [limitIsAssignedTo := true]]. + limitIsAssignedTo ifTrue: + [^false]]. arguments size < 3 ifTrue: "transform to full form" [selector := SelectorNode new key: #to:by:do: code: #macro]. "Now generate auxiliary structures" myRange := encoder rawSourceRanges at: self ifAbsent: [1 to: 0]. block := arguments last. blockRange := encoder rawSourceRanges at: block ifAbsent: [1 to: 0]. blockVar := block firstArgument. initStmt := AssignmentNode new variable: blockVar value: receiver. - limit := arguments at: 1. limit isVariableReference | limit isConstantNumber ifTrue: [limitInit := nil] ifFalse: "Need to store limit in a var" [limit := encoder bindBlockArg: blockVar key, 'LimiT' within: block. limit scope: -2. "Already done parsing block; flag so it won't print" block addArgument: limit. limitInit := AssignmentNode new variable: limit value: arguments first]. test := MessageNode new receiver: blockVar selector: (increment key > 0 ifTrue: [#<=] ifFalse: [#>=]) arguments: (Array with: limit) precedence: precedence from: encoder sourceRange: (myRange first to: blockRange first). incStmt := AssignmentNode new variable: blockVar value: (MessageNode new receiver: blockVar selector: #+ arguments: (Array with: increment) precedence: precedence from: encoder) from: encoder sourceRange: (myRange last to: myRange last). arguments := (Array with: limit with: increment with: block), (Array with: initStmt with: test with: incStmt with: limitInit). block noteOptimizedIn: self. ^true! |
On Tue, Sep 7, 2010 at 4:36 PM, <[hidden email]> wrote: Nicolas Cellier uploaded a new version of Compiler to project The Trunk: Right. I considered this and immediately rejected it on the grounds that, because it uses an extra temp and we're severely limited on the max number of temps, therefore the shadow variable approach would very likely cause failed compilations. Since assigning to the loop variable is an edge case anyway I think the approach I've taken is the better one.
|
2010/9/8 Eliot Miranda <[hidden email]>:
> > > On Tue, Sep 7, 2010 at 4:36 PM, <[hidden email]> wrote: >> >> Nicolas Cellier uploaded a new version of Compiler to project The Trunk: >> http://source.squeak.org/trunk/Compiler-nice.173.mcz >> >> ==================== Summary ==================== >> >> Name: Compiler-nice.173 >> Author: nice >> Time: 8 September 2010, 1:36:04.973 am >> UUID: f40f6937-a88a-4f85-b0b0-f68b310a1d92 >> Ancestors: Compiler-eem.172 >> >> Compiler fix for http://bugs.squeak.org/view.php?id=7093 >> Avoid optimizing a to:do: loop that does modify its limit inside the loop. >> Thanks Eliot! >> >> Note: an alternate solution would be to create a shadow variable, assign >> it with the limit, and optimize the block. > > Right. I considered this and immediately rejected it on the grounds that, > because it uses an extra temp and we're severely limited on the max number > of temps, therefore the shadow variable approach would very likely cause > failed compilations. Since assigning to the loop variable is an edge case > anyway I think the approach I've taken is the better one. Fully agree >> >> Note 2: the bug report also has a so far uncorrected Decompiler part and >> cannot be closed yet. >> >> =============== Diff against Compiler-eem.172 =============== >> >> Item was changed: >> ----- Method: MessageNode>>transformToDo: (in category 'macro >> transformations') ----- >> transformToDo: encoder >> " var := rcvr. L1: [var <= arg1] Bfp(L2) [block body. var := var + >> inc] >> Jmp(L1) L2: " >> + | limit increment block initStmt test incStmt limitInit blockVar >> myRange blockRange limitIsAssignedTo | >> - | limit increment block initStmt test incStmt limitInit blockVar >> myRange blockRange | >> "First check for valid arguments" >> ((arguments last isMemberOf: BlockNode) >> and: [arguments last numberOfArguments = 1 >> and: [arguments last firstArgument isVariableReference "As with >> debugger remote vars"]]) ifFalse: >> [^false]. >> arguments size = 3 >> ifTrue: [increment := arguments at: 2. >> (increment isConstantNumber >> + and: [increment literalValue ~= 0]) >> ifFalse: [^false]] >> - and: [increment literalValue ~= 0]) >> ifFalse: [^ false]] >> ifFalse: [increment := encoder encodeLiteral: 1]. >> + (limit := arguments at: 1) isVariableReference ifTrue: >> + [limitIsAssignedTo := false. >> + arguments last nodesDo: >> + [:node| >> + (node isAssignmentNode and: [node variable = >> limit]) ifTrue: >> + [limitIsAssignedTo := true]]. >> + limitIsAssignedTo ifTrue: >> + [^false]]. >> arguments size < 3 ifTrue: "transform to full form" >> [selector := SelectorNode new key: #to:by:do: code: >> #macro]. >> >> "Now generate auxiliary structures" >> myRange := encoder rawSourceRanges at: self ifAbsent: [1 to: 0]. >> block := arguments last. >> blockRange := encoder rawSourceRanges at: block ifAbsent: [1 to: >> 0]. >> blockVar := block firstArgument. >> initStmt := AssignmentNode new variable: blockVar value: receiver. >> - limit := arguments at: 1. >> limit isVariableReference | limit isConstantNumber >> ifTrue: [limitInit := nil] >> ifFalse: "Need to store limit in a var" >> [limit := encoder bindBlockArg: blockVar key, >> 'LimiT' within: block. >> limit scope: -2. "Already done parsing block; >> flag so it won't print" >> block addArgument: limit. >> limitInit := AssignmentNode new >> variable: limit >> value: arguments >> first]. >> test := MessageNode new >> receiver: blockVar >> selector: (increment key > 0 ifTrue: [#<=] >> ifFalse: [#>=]) >> arguments: (Array with: limit) >> precedence: precedence from: encoder >> sourceRange: (myRange first to: blockRange >> first). >> incStmt := AssignmentNode new >> variable: blockVar >> value: (MessageNode new >> receiver: blockVar >> selector: #+ >> arguments: (Array >> with: increment) >> precedence: >> precedence from: encoder) >> from: encoder >> sourceRange: (myRange last to: myRange >> last). >> arguments := (Array with: limit with: increment with: block), >> (Array with: initStmt with: test >> with: incStmt with: limitInit). >> block noteOptimizedIn: self. >> ^true! >> >> > > > > > |
Free forum by Nabble | Edit this page |