From 2aaaf30db74f350926a0c34ebac9820111b91997 Mon Sep 17 00:00:00 2001 From: Evgeniy Stratonikov Date: Wed, 28 Apr 2021 11:20:41 +0300 Subject: [PATCH 1/5] compiler: remove `finallyProcessIndex` from `funcScope` --- pkg/compiler/analysis.go | 2 +- pkg/compiler/codegen.go | 8 +++++--- pkg/compiler/func_scope.go | 9 +++++---- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/pkg/compiler/analysis.go b/pkg/compiler/analysis.go index 112c6945d..a28a8386a 100644 --- a/pkg/compiler/analysis.go +++ b/pkg/compiler/analysis.go @@ -121,7 +121,7 @@ func (c *codegen) traverseGlobals() (int, int, int) { // store auxiliary variables after all others. if hasDefer { c.exceptionIndex = len(c.globals) - c.globals[""] = c.exceptionIndex + c.globals[exceptionVarName] = c.exceptionIndex } } return n, initLocals, deployLocals diff --git a/pkg/compiler/codegen.go b/pkg/compiler/codegen.go index 81d5dedcd..7822d1e0d 100644 --- a/pkg/compiler/codegen.go +++ b/pkg/compiler/codegen.go @@ -1296,7 +1296,9 @@ func (c *codegen) processDefers() { c.setLabel(stmt.catchLabel) c.emitStoreByIndex(varGlobal, c.exceptionIndex) emit.Int(c.prog.BinWriter, 1) - c.emitStoreByIndex(varLocal, c.scope.finallyProcessedIndex) + + finalIndex := c.getVarIndex("", finallyVarName).index + c.emitStoreByIndex(varLocal, finalIndex) ast.Walk(c, stmt.expr) if i == 0 { // After panic, default values must be returns, except for named returns, @@ -1309,12 +1311,12 @@ func (c *codegen) processDefers() { c.setLabel(stmt.finallyLabel) before := c.newLabel() - c.emitLoadByIndex(varLocal, c.scope.finallyProcessedIndex) + c.emitLoadByIndex(varLocal, finalIndex) emit.Jmp(c.prog.BinWriter, opcode.JMPIFL, before) ast.Walk(c, stmt.expr) c.setLabel(before) emit.Int(c.prog.BinWriter, 0) - c.emitStoreByIndex(varLocal, c.scope.finallyProcessedIndex) + c.emitStoreByIndex(varLocal, finalIndex) emit.Opcodes(c.prog.BinWriter, opcode.ENDFINALLY) c.setLabel(after) } diff --git a/pkg/compiler/func_scope.go b/pkg/compiler/func_scope.go index 672b839ef..e09d7a6b9 100644 --- a/pkg/compiler/func_scope.go +++ b/pkg/compiler/func_scope.go @@ -34,9 +34,6 @@ type funcScope struct { // deferStack is a stack containing encountered `defer` statements. deferStack []deferInfo - // finallyProcessed is a index of static slot with boolean flag determining - // if `defer` statement was already processed. - finallyProcessedIndex int // Local variables vars varScope @@ -59,6 +56,11 @@ type deferInfo struct { expr *ast.CallExpr } +const ( + finallyVarName = "" + exceptionVarName = "" +) + func (c *codegen) newFuncScope(decl *ast.FuncDecl, label uint16) *funcScope { var name string if decl.Name != nil { @@ -204,7 +206,6 @@ func (c *codegen) countLocalsInline(decl *ast.FuncDecl, pkg *types.Package, f *f func (c *codegen) countLocalsWithDefer(f *funcScope) int { size, hasDefer := c.countLocals(f.decl) if hasDefer { - f.finallyProcessedIndex = size size++ } return size From bfa2bafb04a62b0ceb8f5c47be8ae10098cd08e3 Mon Sep 17 00:00:00 2001 From: Evgeniy Stratonikov Date: Wed, 28 Apr 2021 15:52:40 +0300 Subject: [PATCH 2/5] compiler: optimize jumps in tests Jump target shortening affects manifest, so we better be sure. --- pkg/compiler/debug_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/compiler/debug_test.go b/pkg/compiler/debug_test.go index b5dfbd121..fbcef22d3 100644 --- a/pkg/compiler/debug_test.go +++ b/pkg/compiler/debug_test.go @@ -73,7 +73,9 @@ func _deploy(data interface{}, isUpdate bool) { x := 1; _ = x } c := newCodegen(info, pkg) require.NoError(t, c.compile(info, pkg)) - buf := c.prog.Bytes() + buf, err := c.writeJumps(c.prog.Bytes()) + require.NoError(t, err) + d := c.emitDebugInfo(buf) require.NotNil(t, d) From b693d542825b92de4d32c334b620e5d06c516833 Mon Sep 17 00:00:00 2001 From: Evgeniy Stratonikov Date: Wed, 28 Apr 2021 16:10:44 +0300 Subject: [PATCH 3/5] compiler: count local variables on the go Create local variables as they are needed and remove `INITSLOT` if there were no locals. This helps to eliminate a whole class of bugs when calculated and real amount mismatched. --- cli/nep17_test.go | 2 +- cli/testdata/wallet1_solo.json | 4 +- pkg/compiler/analysis.go | 129 ++++++++++++++-------------- pkg/compiler/codegen.go | 132 +++++++++++++++++++---------- pkg/compiler/defer_test.go | 11 +++ pkg/compiler/func_scope.go | 108 ----------------------- pkg/compiler/function_call_test.go | 5 +- pkg/compiler/inline_test.go | 18 ++-- pkg/compiler/syscall_test.go | 2 + 9 files changed, 181 insertions(+), 230 deletions(-) diff --git a/cli/nep17_test.go b/cli/nep17_test.go index 1c6984814..ff9e65408 100644 --- a/cli/nep17_test.go +++ b/cli/nep17_test.go @@ -86,7 +86,7 @@ func TestNEP17Balance(t *testing.T) { } e.checkNextLine(t, "^\\s*$") - addr4, err := address.StringToUint160("NQKpygA5oG8KRivZeYjXVU2T1ZPmUmaqQF") // deployed verify.go contract + addr4, err := address.StringToUint160("NU4CTk9H2fgNCuC3ZPqX4LjUX3MHt3Rh6p") // deployed verify.go contract require.NoError(t, err) e.checkNextLine(t, "^Account "+address.Uint160ToString(addr4)) e.checkEOF(t) diff --git a/cli/testdata/wallet1_solo.json b/cli/testdata/wallet1_solo.json index 40c719cba..6cc20c33b 100644 --- a/cli/testdata/wallet1_solo.json +++ b/cli/testdata/wallet1_solo.json @@ -63,9 +63,9 @@ "key" : "6PYM8VdX2BSm7BSXKzV4Fz6S3R9cDLLWNrD9nMjxW352jEv3fsC8N3wNLY" }, { - "address" : "NQKpygA5oG8KRivZeYjXVU2T1ZPmUmaqQF", + "address" : "NU4CTk9H2fgNCuC3ZPqX4LjUX3MHt3Rh6p", "contract" : { - "script" : "VwEAEdsgQFcAA0BXAQR4eXp7VBTAcAwOT25ORVAxMVBheW1lbnRoUEGVAW9hIUA=", + "script" : "EdsgQFcAA0BXAQR4eXp7VBTAcAwOT25ORVAxMVBheW1lbnRoUEGVAW9hIUA=", "parameters" : [], "deployed" : true }, diff --git a/pkg/compiler/analysis.go b/pkg/compiler/analysis.go index a28a8386a..e9ead23b9 100644 --- a/pkg/compiler/analysis.go +++ b/pkg/compiler/analysis.go @@ -37,46 +37,20 @@ func (c *codegen) getIdentName(pkg string, name string) string { } // traverseGlobals visits and initializes global variables. -// and returns number of variables initialized. -// Second return value is -1 if no `init()` functions were encountered -// and number of maximum amount of locals in any of init functions otherwise. -// Same for `_deploy()` functions (see docs/compiler.md). -func (c *codegen) traverseGlobals() (int, int, int) { +// It returns `true` if contract has `_deploy` function. +func (c *codegen) traverseGlobals() bool { var hasDefer bool var n, nConst int - initLocals := -1 - deployLocals := -1 + var hasDeploy bool c.ForEachFile(func(f *ast.File, pkg *types.Package) { nv, nc := countGlobals(f) n += nv nConst += nc - if initLocals == -1 || deployLocals == -1 || !hasDefer { + if !hasDeploy || !hasDefer { ast.Inspect(f, func(node ast.Node) bool { switch n := node.(type) { - case *ast.GenDecl: - if n.Tok == token.VAR { - for i := range n.Specs { - for _, v := range n.Specs[i].(*ast.ValueSpec).Values { - num := c.countLocalsCall(v, pkg) - if num > initLocals { - initLocals = num - } - } - } - } case *ast.FuncDecl: - if isInitFunc(n) { - num, _ := c.countLocals(n) - if num > initLocals { - initLocals = num - } - } else if isDeployFunc(n) { - num, _ := c.countLocals(n) - if num > deployLocals { - deployLocals = num - } - } - return !hasDefer + hasDeploy = hasDeploy || isDeployFunc(n) case *ast.DeferStmt: hasDefer = true return false @@ -88,43 +62,70 @@ func (c *codegen) traverseGlobals() (int, int, int) { if hasDefer { n++ } - if n+nConst != 0 || initLocals > -1 { - if n > 255 { - c.prog.BinWriter.Err = errors.New("too many global variables") - return 0, initLocals, deployLocals - } - if n != 0 { - emit.Instruction(c.prog.BinWriter, opcode.INITSSLOT, []byte{byte(n)}) - } - if initLocals > 0 { - emit.Instruction(c.prog.BinWriter, opcode.INITSLOT, []byte{byte(initLocals), 0}) - } - seenBefore := false - c.ForEachPackage(func(pkg *loader.PackageInfo) { - if n+nConst > 0 { - for _, f := range pkg.Files { - c.fillImportMap(f, pkg.Pkg) - c.convertGlobals(f, pkg.Pkg) - } + + if n > 255 { + c.prog.BinWriter.Err = errors.New("too many global variables") + return hasDeploy + } + + if n != 0 { + emit.Instruction(c.prog.BinWriter, opcode.INITSSLOT, []byte{byte(n)}) + } + + initOffset := c.prog.Len() + emit.Instruction(c.prog.BinWriter, opcode.INITSLOT, []byte{0, 0}) + + lastCnt, maxCnt := -1, -1 + c.ForEachPackage(func(pkg *loader.PackageInfo) { + if n+nConst > 0 { + for _, f := range pkg.Files { + c.fillImportMap(f, pkg.Pkg) + c.convertGlobals(f, pkg.Pkg) } - if initLocals > -1 { - for _, f := range pkg.Files { - c.fillImportMap(f, pkg.Pkg) - seenBefore = c.convertInitFuncs(f, pkg.Pkg, seenBefore) || seenBefore - } + } + for _, f := range pkg.Files { + c.fillImportMap(f, pkg.Pkg) + + var currMax int + lastCnt, currMax = c.convertInitFuncs(f, pkg.Pkg, lastCnt) + if currMax > maxCnt { + maxCnt = currMax + } + } + // because we reuse `convertFuncDecl` for init funcs, + // we need to cleare scope, so that global variables + // encountered after will be recognized as globals. + c.scope = nil + }) + + // Here we remove `INITSLOT` if no code was emitted for `init` function. + // Note that the `INITSSLOT` must stay in place. + hasNoInit := initOffset+3 == c.prog.Len() + if hasNoInit { + buf := c.prog.Bytes() + c.prog.Reset() + c.prog.WriteBytes(buf[:initOffset]) + } + + if initOffset != 0 || !hasNoInit { // if there are some globals or `init()`. + c.initEndOffset = c.prog.Len() + emit.Opcodes(c.prog.BinWriter, opcode.RET) + + if maxCnt >= 0 { + c.reverseOffsetMap[initOffset] = nameWithLocals{ + name: "init", + count: maxCnt, } - // because we reuse `convertFuncDecl` for init funcs, - // we need to cleare scope, so that global variables - // encountered after will be recognized as globals. - c.scope = nil - }) - // store auxiliary variables after all others. - if hasDefer { - c.exceptionIndex = len(c.globals) - c.globals[exceptionVarName] = c.exceptionIndex } } - return n, initLocals, deployLocals + + // store auxiliary variables after all others. + if hasDefer { + c.exceptionIndex = len(c.globals) + c.globals[exceptionVarName] = c.exceptionIndex + } + + return hasDeploy } // countGlobals counts the global variables in the program to add diff --git a/pkg/compiler/codegen.go b/pkg/compiler/codegen.go index 7822d1e0d..6081eda8a 100644 --- a/pkg/compiler/codegen.go +++ b/pkg/compiler/codegen.go @@ -40,6 +40,9 @@ type codegen struct { // A mapping of lambda functions into their scope. lambda map[string]*funcScope + // reverseOffsetMap maps function offsets to a local variable count. + reverseOffsetMap map[int]nameWithLocals + // Current funcScope being converted. scope *funcScope @@ -124,6 +127,11 @@ type labelWithStackSize struct { sz int } +type nameWithLocals struct { + name string + count int +} + type varType int const ( @@ -333,24 +341,30 @@ func (c *codegen) clearSlots(n int) { } } -func (c *codegen) convertInitFuncs(f *ast.File, pkg *types.Package, seenBefore bool) bool { +// convertInitFuncs converts `init()` functions in file f and returns +// number of locals in last processed definition as well as maximum locals number encountered. +func (c *codegen) convertInitFuncs(f *ast.File, pkg *types.Package, lastCount int) (int, int) { + maxCount := -1 ast.Inspect(f, func(node ast.Node) bool { switch n := node.(type) { case *ast.FuncDecl: if isInitFunc(n) { - if seenBefore { - cnt, _ := c.countLocals(n) - c.clearSlots(cnt) - seenBefore = true + if lastCount != -1 { + c.clearSlots(lastCount) + } + + f := c.convertFuncDecl(f, n, pkg) + lastCount = f.vars.localsCnt + if lastCount > maxCount { + maxCount = lastCount } - c.convertFuncDecl(f, n, pkg) } case *ast.GenDecl: return false } return true }) - return seenBefore + return lastCount, maxCount } func isDeployFunc(decl *ast.FuncDecl) bool { @@ -363,19 +377,22 @@ func isDeployFunc(decl *ast.FuncDecl) bool { return ok && typ.Name == "bool" } -func (c *codegen) convertDeployFuncs() { - seenBefore := false +func (c *codegen) convertDeployFuncs() int { + maxCount, lastCount := 0, -1 c.ForEachFile(func(f *ast.File, pkg *types.Package) { ast.Inspect(f, func(node ast.Node) bool { switch n := node.(type) { case *ast.FuncDecl: if isDeployFunc(n) { - if seenBefore { - cnt, _ := c.countLocals(n) - c.clearSlots(cnt) + if lastCount != -1 { + c.clearSlots(lastCount) + } + + f := c.convertFuncDecl(f, n, pkg) + lastCount = f.vars.localsCnt + if lastCount > maxCount { + maxCount = lastCount } - c.convertFuncDecl(f, n, pkg) - seenBefore = true } case *ast.GenDecl: return false @@ -383,9 +400,10 @@ func (c *codegen) convertDeployFuncs() { return true }) }) + return maxCount } -func (c *codegen) convertFuncDecl(file ast.Node, decl *ast.FuncDecl, pkg *types.Package) { +func (c *codegen) convertFuncDecl(file ast.Node, decl *ast.FuncDecl, pkg *types.Package) *funcScope { var ( f *funcScope ok, isLambda bool @@ -399,7 +417,7 @@ func (c *codegen) convertFuncDecl(file ast.Node, decl *ast.FuncDecl, pkg *types. if ok { // If this function is a syscall or builtin we will not convert it to bytecode. if isSyscall(f) || isCustomBuiltin(f) { - return + return f } c.setLabel(f.label) } else if f, ok = c.lambda[c.getIdentName("", decl.Name.Name)]; ok { @@ -417,17 +435,11 @@ func (c *codegen) convertFuncDecl(file ast.Node, decl *ast.FuncDecl, pkg *types. // All globals copied into the scope of the function need to be added // to the stack size of the function. if !isInit && !isDeploy { - sizeLoc := c.countLocalsWithDefer(f) - if sizeLoc > 255 { - c.prog.Err = errors.New("maximum of 255 local variables is allowed") - } sizeArg := f.countArgs() if sizeArg > 255 { c.prog.Err = errors.New("maximum of 255 local variables is allowed") } - if sizeLoc != 0 || sizeArg != 0 { - emit.Instruction(c.prog.BinWriter, opcode.INITSLOT, []byte{byte(sizeLoc), byte(sizeArg)}) - } + emit.Instruction(c.prog.BinWriter, opcode.INITSLOT, []byte{byte(0), byte(sizeArg)}) } f.vars.newScope() @@ -478,6 +490,14 @@ func (c *codegen) convertFuncDecl(file ast.Node, decl *ast.FuncDecl, pkg *types. } c.lambda = make(map[string]*funcScope) } + + if !isInit && !isDeploy { + c.reverseOffsetMap[int(f.rng.Start)] = nameWithLocals{ + name: f.name, + count: f.vars.localsCnt, + } + } + return f } func (c *codegen) Visit(node ast.Node) ast.Visitor { @@ -1961,17 +1981,16 @@ func (c *codegen) compile(info *buildInfo, pkg *loader.PackageInfo) error { // Bring all imported functions into scope. c.ForEachFile(c.resolveFuncDecls) - n, initLocals, deployLocals := c.traverseGlobals() - hasInit := initLocals > -1 - if n > 0 || hasInit { - c.initEndOffset = c.prog.Len() - emit.Opcodes(c.prog.BinWriter, opcode.RET) - } + hasDeploy := c.traverseGlobals() - hasDeploy := deployLocals > -1 if hasDeploy { - emit.Instruction(c.prog.BinWriter, opcode.INITSLOT, []byte{byte(deployLocals), 2}) - c.convertDeployFuncs() + deployOffset := c.prog.Len() + emit.Instruction(c.prog.BinWriter, opcode.INITSLOT, []byte{0, 2}) + locCount := c.convertDeployFuncs() + c.reverseOffsetMap[deployOffset] = nameWithLocals{ + name: "_deploy", + count: locCount, + } c.deployEndOffset = c.prog.Len() emit.Opcodes(c.prog.BinWriter, opcode.RET) } @@ -2008,16 +2027,17 @@ func (c *codegen) compile(info *buildInfo, pkg *loader.PackageInfo) error { func newCodegen(info *buildInfo, pkg *loader.PackageInfo) *codegen { return &codegen{ - buildInfo: info, - prog: io.NewBufBinWriter(), - l: []int{}, - funcs: map[string]*funcScope{}, - lambda: map[string]*funcScope{}, - globals: map[string]int{}, - labels: map[labelWithType]uint16{}, - typeInfo: &pkg.Info, - constMap: map[string]types.TypeAndValue{}, - docIndex: map[string]int{}, + buildInfo: info, + prog: io.NewBufBinWriter(), + l: []int{}, + funcs: map[string]*funcScope{}, + lambda: map[string]*funcScope{}, + reverseOffsetMap: map[int]nameWithLocals{}, + globals: map[string]int{}, + labels: map[labelWithType]uint16{}, + typeInfo: &pkg.Info, + constMap: map[string]types.TypeAndValue{}, + docIndex: map[string]int{}, initEndOffset: -1, deployEndOffset: -1, @@ -2087,6 +2107,18 @@ func (c *codegen) writeJumps(b []byte) ([]byte, error) { if op != opcode.PUSHA && math.MinInt8 <= offset && offset <= math.MaxInt8 { offsets = append(offsets, ctx.IP()) } + case opcode.INITSLOT: + nextIP := ctx.NextIP() + info := c.reverseOffsetMap[ctx.IP()] + if argCount := b[nextIP-1]; info.count == 0 && argCount == 0 { + offsets = append(offsets, ctx.IP()) + continue + } + + if info.count > 255 { + return nil, fmt.Errorf("func '%s' has %d local variables (maximum is 255)", info.name, info.count) + } + b[nextIP-2] = byte(info.count) } } @@ -2139,6 +2171,7 @@ func (c *codegen) replaceLabelWithOffset(ip int, arg []byte) (int, error) { } // longToShortRemoveCount is a difference between short and long instruction sizes in bytes. +// By pure coincidence, this is also the size of `INITSLOT` instruction. const longToShortRemoveCount = 3 // shortenJumps returns converts b to a program where all long JMP*/CALL* specified by absolute offsets, @@ -2196,13 +2229,22 @@ func shortenJumps(b []byte, offsets []int) []byte { // 2. Convert instructions. copyOffset := 0 l := len(offsets) - b[offsets[0]] = byte(toShortForm(opcode.Opcode(b[offsets[0]]))) + if op := opcode.Opcode(b[offsets[0]]); op != opcode.INITSLOT { + b[offsets[0]] = byte(toShortForm(op)) + } for i := 0; i < l; i++ { start := offsets[i] + 2 + if b[offsets[i]] == byte(opcode.INITSLOT) { + start = offsets[i] + } + end := len(b) if i != l-1 { - end = offsets[i+1] + 2 - b[offsets[i+1]] = byte(toShortForm(opcode.Opcode(b[offsets[i+1]]))) + end = offsets[i+1] + if op := opcode.Opcode(b[offsets[i+1]]); op != opcode.INITSLOT { + end += 2 + b[offsets[i+1]] = byte(toShortForm(op)) + } } copy(b[start-copyOffset:], b[start+3:end]) copyOffset += longToShortRemoveCount diff --git a/pkg/compiler/defer_test.go b/pkg/compiler/defer_test.go index d9b39ea07..2cabbed13 100644 --- a/pkg/compiler/defer_test.go +++ b/pkg/compiler/defer_test.go @@ -137,3 +137,14 @@ func TestRecover(t *testing.T) { eval(t, src, big.NewInt(5)) }) } + +func TestDeferNoGlobals(t *testing.T) { + src := `package foo + func Main() int { + a := 1 + defer func() { recover() }() + panic("msg") + return a + }` + eval(t, src, big.NewInt(0)) +} diff --git a/pkg/compiler/func_scope.go b/pkg/compiler/func_scope.go index e09d7a6b9..e466f8bec 100644 --- a/pkg/compiler/func_scope.go +++ b/pkg/compiler/func_scope.go @@ -2,7 +2,6 @@ package compiler import ( "go/ast" - "go/token" "go/types" ) @@ -104,113 +103,6 @@ func (c *funcScope) analyzeVoidCalls(node ast.Node) bool { return true } -func (c *codegen) countLocalsCall(n ast.Expr, pkg *types.Package) int { - ce, ok := n.(*ast.CallExpr) - if !ok { - return -1 - } - - var size int - var name string - switch fun := ce.Fun.(type) { - case *ast.Ident: - var pkgName string - if pkg != nil { - pkgName = pkg.Path() - } - name = c.getIdentName(pkgName, fun.Name) - case *ast.SelectorExpr: - name, _ = c.getFuncNameFromSelector(fun) - default: - return 0 - } - if inner, ok := c.funcs[name]; ok && canInline(name) { - sig, ok := c.typeOf(ce.Fun).(*types.Signature) - if !ok { - info := c.buildInfo.program.Package(pkg.Path()) - sig = info.Types[ce.Fun].Type.(*types.Signature) - } - for i := range ce.Args { - switch ce.Args[i].(type) { - case *ast.Ident: - case *ast.BasicLit: - default: - size++ - } - } - // Variadic with direct var args. - if sig.Variadic() && !ce.Ellipsis.IsValid() { - size++ - } - innerSz, _ := c.countLocalsInline(inner.decl, inner.pkg, inner) - size += innerSz - } - return size -} - -func (c *codegen) countLocals(decl *ast.FuncDecl) (int, bool) { - return c.countLocalsInline(decl, nil, nil) -} - -func (c *codegen) countLocalsInline(decl *ast.FuncDecl, pkg *types.Package, f *funcScope) (int, bool) { - oldMap := c.importMap - if pkg != nil { - c.fillImportMap(f.file, pkg) - } - - size := 0 - hasDefer := false - ast.Inspect(decl, func(n ast.Node) bool { - switch n := n.(type) { - case *ast.CallExpr: - size += c.countLocalsCall(n, pkg) - case *ast.FuncType: - num := n.Results.NumFields() - if num != 0 && len(n.Results.List[0].Names) != 0 { - size += num - } - case *ast.AssignStmt: - if n.Tok == token.DEFINE { - size += len(n.Lhs) - } - case *ast.DeferStmt: - hasDefer = true - return false - case *ast.ReturnStmt: - if pkg == nil { - size++ - } - case *ast.IfStmt: - size++ - // This handles the inline GenDecl like "var x = 2" - case *ast.ValueSpec: - size += len(n.Names) - case *ast.RangeStmt: - if n.Tok == token.DEFINE { - if n.Key != nil { - size++ - } - if n.Value != nil { - size++ - } - } - } - return true - }) - if pkg != nil { - c.importMap = oldMap - } - return size, hasDefer -} - -func (c *codegen) countLocalsWithDefer(f *funcScope) int { - size, hasDefer := c.countLocals(f.decl) - if hasDefer { - size++ - } - return size -} - func (c *funcScope) countArgs() int { n := c.decl.Type.Params.NumFields() if c.decl.Recv != nil { diff --git a/pkg/compiler/function_call_test.go b/pkg/compiler/function_call_test.go index ef2fd8c61..60935cbbe 100644 --- a/pkg/compiler/function_call_test.go +++ b/pkg/compiler/function_call_test.go @@ -306,7 +306,10 @@ func TestJumpOptimize(t *testing.T) { require.NoError(t, err) require.Equal(t, 6, len(di.Methods)) for _, mi := range di.Methods { - require.Equal(t, b[mi.Range.Start], byte(opcode.INITSLOT)) + // only _deploy and init have locals here + if mi.Name.Name == "_deploy" || mi.Name.Name == "init" { + require.Equal(t, b[mi.Range.Start], byte(opcode.INITSLOT)) + } require.Equal(t, b[mi.Range.End], byte(opcode.RET)) } } diff --git a/pkg/compiler/inline_test.go b/pkg/compiler/inline_test.go index bf62e65a3..beb84b5b3 100644 --- a/pkg/compiler/inline_test.go +++ b/pkg/compiler/inline_test.go @@ -30,7 +30,7 @@ func checkCallCount(t *testing.T, src string, expectedCall, expectedInitSlot int } } require.Equal(t, expectedCall, actualCall) - require.Equal(t, expectedInitSlot, actualInitSlot) + require.True(t, expectedInitSlot == actualInitSlot) } func TestInline(t *testing.T) { @@ -47,34 +47,34 @@ func TestInline(t *testing.T) { t.Run("no return", func(t *testing.T) { src := fmt.Sprintf(srcTmpl, `inline.NoArgsNoReturn() return 1`) - checkCallCount(t, src, 0, 1) + checkCallCount(t, src, 0, 0) eval(t, src, big.NewInt(1)) }) t.Run("has return, dropped", func(t *testing.T) { src := fmt.Sprintf(srcTmpl, `inline.NoArgsReturn1() return 2`) - checkCallCount(t, src, 0, 1) + checkCallCount(t, src, 0, 0) eval(t, src, big.NewInt(2)) }) t.Run("drop twice", func(t *testing.T) { src := fmt.Sprintf(srcTmpl, `inline.DropInsideInline() return 42`) - checkCallCount(t, src, 0, 1) + checkCallCount(t, src, 0, 0) eval(t, src, big.NewInt(42)) }) t.Run("no args return 1", func(t *testing.T) { src := fmt.Sprintf(srcTmpl, `return inline.NoArgsReturn1()`) - checkCallCount(t, src, 0, 1) + checkCallCount(t, src, 0, 0) eval(t, src, big.NewInt(1)) }) t.Run("sum", func(t *testing.T) { src := fmt.Sprintf(srcTmpl, `return inline.Sum(1, 2)`) - checkCallCount(t, src, 0, 1) + checkCallCount(t, src, 0, 0) eval(t, src, big.NewInt(3)) }) t.Run("sum squared (nested inline)", func(t *testing.T) { src := fmt.Sprintf(srcTmpl, `return inline.SumSquared(1, 2)`) - checkCallCount(t, src, 0, 1) + checkCallCount(t, src, 0, 0) eval(t, src, big.NewInt(9)) }) t.Run("inline function in inline function parameter", func(t *testing.T) { @@ -84,7 +84,7 @@ func TestInline(t *testing.T) { }) t.Run("global name clash", func(t *testing.T) { src := fmt.Sprintf(srcTmpl, `return inline.GetSumSameName()`) - checkCallCount(t, src, 0, 1) + checkCallCount(t, src, 0, 0) eval(t, src, big.NewInt(42)) }) t.Run("local name clash", func(t *testing.T) { @@ -110,7 +110,7 @@ func TestInline(t *testing.T) { }) t.Run("globals", func(t *testing.T) { src := fmt.Sprintf(srcTmpl, `return inline.Concat(Num)`) - checkCallCount(t, src, 0, 1) + checkCallCount(t, src, 0, 0) eval(t, src, big.NewInt(221)) }) } diff --git a/pkg/compiler/syscall_test.go b/pkg/compiler/syscall_test.go index 86adb3306..4a5df2d6e 100644 --- a/pkg/compiler/syscall_test.go +++ b/pkg/compiler/syscall_test.go @@ -191,6 +191,8 @@ func TestNotify(t *testing.T) { } func TestSyscallInGlobalInit(t *testing.T) { + // FIXME(fyrchik): count auxiliary inline locals for INITSLOT in global context + t.Skip() src := `package foo import "github.com/nspcc-dev/neo-go/pkg/interop/runtime" var a = runtime.CheckWitness([]byte("5T")) From 87a69b13f1c17c6a87cd96d73453dc7a48f3501e Mon Sep 17 00:00:00 2001 From: Evgeniy Stratonikov Date: Wed, 19 May 2021 10:34:44 +0300 Subject: [PATCH 4/5] compiler/test: add test for inlining with local alias INITSLOT count should be 1 more than for the same test with global. --- pkg/compiler/inline_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/compiler/inline_test.go b/pkg/compiler/inline_test.go index beb84b5b3..46db2cece 100644 --- a/pkg/compiler/inline_test.go +++ b/pkg/compiler/inline_test.go @@ -113,6 +113,11 @@ func TestInline(t *testing.T) { checkCallCount(t, src, 0, 0) eval(t, src, big.NewInt(221)) }) + t.Run("locals, alias", func(t *testing.T) { + src := fmt.Sprintf(srcTmpl, `num := 1; return inline.Concat(num)`) + checkCallCount(t, src, 0, 1) + eval(t, src, big.NewInt(221)) + }) } func TestInlineInLoop(t *testing.T) { From 60a3e0d778c2baf1e1fcdb8d5496be07f99ff879 Mon Sep 17 00:00:00 2001 From: Evgeniy Stratonikov Date: Wed, 28 Apr 2021 16:37:31 +0300 Subject: [PATCH 5/5] compiler: count auxiliary locals introduced by inlining During initialization of globals no function scope is present thus locals number is not saved anywere. Save it in `codegen` directly. --- pkg/compiler/analysis.go | 4 ++++ pkg/compiler/codegen.go | 3 +++ pkg/compiler/inline.go | 7 ++++++- pkg/compiler/syscall_test.go | 2 -- 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/pkg/compiler/analysis.go b/pkg/compiler/analysis.go index e9ead23b9..019f4a7c9 100644 --- a/pkg/compiler/analysis.go +++ b/pkg/compiler/analysis.go @@ -98,6 +98,10 @@ func (c *codegen) traverseGlobals() bool { c.scope = nil }) + if c.globalInlineCount > maxCnt { + maxCnt = c.globalInlineCount + } + // Here we remove `INITSLOT` if no code was emitted for `init` function. // Note that the `INITSSLOT` must stay in place. hasNoInit := initOffset+3 == c.prog.Len() diff --git a/pkg/compiler/codegen.go b/pkg/compiler/codegen.go index 6081eda8a..c2cf96533 100644 --- a/pkg/compiler/codegen.go +++ b/pkg/compiler/codegen.go @@ -61,6 +61,9 @@ type codegen struct { // inlineLabelOffsets contains size of labelList at the start of inline call processing. // For such calls we need to drop only newly created part of stack. inlineLabelOffsets []int + // globalInlineCount contains amount of auxiliary variables introduced by + // function inlining during global variables initialization. + globalInlineCount int // A label for the for-loop being currently visited. currentFor string diff --git a/pkg/compiler/inline.go b/pkg/compiler/inline.go index f3471863b..d3653941b 100644 --- a/pkg/compiler/inline.go +++ b/pkg/compiler/inline.go @@ -32,7 +32,12 @@ func (c *codegen) inlineCall(f *funcScope, n *ast.CallExpr) { if c.scope == nil { c.scope = &funcScope{} c.scope.vars.newScope() - defer func() { c.scope = nil }() + defer func() { + if cnt := c.scope.vars.localsCnt; cnt > c.globalInlineCount { + c.globalInlineCount = cnt + } + c.scope = nil + }() } // Arguments need to be walked with the current scope, diff --git a/pkg/compiler/syscall_test.go b/pkg/compiler/syscall_test.go index 4a5df2d6e..86adb3306 100644 --- a/pkg/compiler/syscall_test.go +++ b/pkg/compiler/syscall_test.go @@ -191,8 +191,6 @@ func TestNotify(t *testing.T) { } func TestSyscallInGlobalInit(t *testing.T) { - // FIXME(fyrchik): count auxiliary inline locals for INITSLOT in global context - t.Skip() src := `package foo import "github.com/nspcc-dev/neo-go/pkg/interop/runtime" var a = runtime.CheckWitness([]byte("5T"))