interp: fix map assignment from arithmetic operations

The logic to trigger assigment optimizations has been refactored for
clarity, and to exclude assignments to map entries.

Fixes #981.
This commit is contained in:
Marc Vertes
2021-01-18 19:04:05 +01:00
committed by GitHub
parent a64fe5b210
commit 8fa00f826c
2 changed files with 37 additions and 19 deletions

15
_test/issue-981.go Normal file
View File

@@ -0,0 +1,15 @@
package main
import "fmt"
func main() {
dp := make(map[int]int)
dp[0] = 1
for i := 1; i < 10; i++ {
dp[i] = dp[i-1] + dp[i-2]
}
fmt.Printf("%v\n", dp)
}
// Output:
// map[0:1 1:1 2:2 3:3 4:5 5:8 6:13 7:21 8:34 9:55]

View File

@@ -555,10 +555,21 @@ func (interp *Interpreter) cfg(root *node, importPath string) ([]*node, error) {
n.findex = dest.findex
n.level = dest.level
// Propagate type
// TODO: Check that existing destination type matches source type
// Propagate type.
// TODO: Check that existing destination type matches source type.
// In the following, we attempt to optimize by skipping the assign
// operation and setting the source location directly to the destination
// location in the frame.
//
switch {
case n.action == aAssign && isCall(src) && dest.typ.cat != interfaceT && !isMapEntry(dest) && !isRecursiveField(dest):
case n.action != aAssign:
// Do not optimize assign combined with another operator.
case isMapEntry(dest):
// Setting a map entry needs an additional step, do not optimize.
// As we only write, skip the default useless getIndexMap dest action.
dest.gen = nop
case isCall(src) && dest.typ.cat != interfaceT && !isRecursiveField(dest):
// Call action may perform the assignment directly.
n.gen = nop
src.level = level
@@ -566,32 +577,25 @@ func (interp *Interpreter) cfg(root *node, importPath string) ([]*node, error) {
if src.typ.untyped && !dest.typ.untyped {
src.typ = dest.typ
}
case n.action == aAssign && src.action == aRecv:
case src.action == aRecv:
// Assign by reading from a receiving channel.
n.gen = nop
src.findex = dest.findex // Set recv address to LHS
src.findex = dest.findex // Set recv address to LHS.
dest.typ = src.typ
case n.action == aAssign && src.action == aCompositeLit && !isMapEntry(dest):
case src.action == aCompositeLit:
if dest.typ.cat == valueT && dest.typ.rtype.Kind() == reflect.Interface {
// Skip optimisation for assigned binary interface or map entry
// which require and additional operation to set the value
// Skip optimisation for assigned interface.
break
}
if dest.action == aGetIndex {
// optimization does not work when assigning to a struct field. Maybe we're not
// setting the right frame index or something, and we would end up not writing at
// the right place. So disabling it for now.
// Optimization does not work when assigning to a struct field.
break
}
// Skip the assign operation entirely, the source frame index is set
// to destination index, avoiding extra memory alloc and duplication.
n.gen = nop
src.findex = dest.findex
src.level = level
case n.action == aAssign && len(n.child) < 4 && !src.rval.IsValid() && isArithmeticAction(src):
case len(n.child) < 4 && !src.rval.IsValid() && isArithmeticAction(src):
// Optimize single assignments from some arithmetic operations.
// Skip the assign operation entirely, the source frame index is set
// to destination index, avoiding extra memory alloc and duplication.
src.typ = dest.typ
src.findex = dest.findex
src.level = level
@@ -600,15 +604,14 @@ func (interp *Interpreter) cfg(root *node, importPath string) ([]*node, error) {
// Assign to nil.
src.rval = reflect.New(dest.typ.TypeOf()).Elem()
}
n.typ = dest.typ
if sym != nil {
sym.typ = n.typ
sym.recv = src.recv
}
n.level = level
if isMapEntry(dest) {
dest.gen = nop // skip getIndexMap
}
if n.anc.kind == constDecl {
n.gen = nop
n.findex = notInFrame