The Trunk: Compiler-nice.173.mcz

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

The Trunk: Compiler-nice.173.mcz

commits-2
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!


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Compiler-nice.173.mcz

Eliot Miranda-2


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.


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!





Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Compiler-nice.173.mcz

Nicolas Cellier
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!
>>
>>
>
>
>
>
>