From 72d0907790fa174f33a90548eb97dafedf4e0384 Mon Sep 17 00:00:00 2001 From: jonny rhea <5555162+jrhea@users.noreply.github.com> Date: Fri, 19 Jun 2026 15:25:01 -0500 Subject: [PATCH] core/vm: replace the stack-helper expansion regexes with an AST inliner --- core/vm/gen/main.go | 300 ++++++++++++++++++++++++++++++------- core/vm/interp_gen.go | 70 ++++----- core/vm/interp_gen_test.go | 95 +++++++----- core/vm/stack.go | 9 ++ 4 files changed, 345 insertions(+), 129 deletions(-) diff --git a/core/vm/gen/main.go b/core/vm/gen/main.go index 6617fb8220..ebb9c12a4b 100644 --- a/core/vm/gen/main.go +++ b/core/vm/gen/main.go @@ -109,10 +109,11 @@ type opMeta struct { } type generator struct { - fset *token.FileSet - handlers map[string]*ast.FuncDecl - meta [256]opMeta - buf *bytes.Buffer + fset *token.FileSet + handlers map[string]*ast.FuncDecl // opXxx handlers from instructions.go and eips.go + stackHelpers map[string]*ast.FuncDecl // (s *Stack) helpers from stack.go, spliced inline + meta [256]opMeta + buf *bytes.Buffer } // p is the sole writer of the generated file. Every line of output is appended @@ -133,12 +134,14 @@ func (g *generator) p(format string, args ...any) { // Handler parsing + body splicing // --------------------------------------------------------------------------- -// parseHandlers parses instructions.go and eips.go and returns every top-level -// opXxx function declaration by name. -func parseHandlers(vmDir string) (*token.FileSet, map[string]*ast.FuncDecl) { - fset := token.NewFileSet() - handlers := map[string]*ast.FuncDecl{} - for _, name := range []string{"instructions.go", "eips.go"} { +// parseHandlers parses instructions.go, eips.go and stack.go. It returns the +// top-level opXxx handlers by name, and separately the *Stack helper methods by +// name (the inliner splices the latter into the former). +func parseHandlers(vmDir string) (fset *token.FileSet, handlers, stackHelpers map[string]*ast.FuncDecl) { + fset = token.NewFileSet() + handlers = map[string]*ast.FuncDecl{} + stackHelpers = map[string]*ast.FuncDecl{} + for _, name := range []string{"instructions.go", "eips.go", "stack.go"} { path := filepath.Join(vmDir, name) f, err := parser.ParseFile(fset, path, nil, parser.ParseComments) if err != nil { @@ -146,13 +149,45 @@ func parseHandlers(vmDir string) (*token.FileSet, map[string]*ast.FuncDecl) { } for _, decl := range f.Decls { fn, ok := decl.(*ast.FuncDecl) - if !ok || fn.Recv != nil || fn.Body == nil { + if !ok || fn.Body == nil { continue } - handlers[fn.Name.Name] = fn + switch { + case fn.Recv == nil: // top-level opXxx handler + handlers[fn.Name.Name] = fn + case isStackMethod(fn) && hasInlineMarker(fn): // (s *Stack) helper tagged //gen:inline + stackHelpers[fn.Name.Name] = fn + } } } - return fset, handlers + return fset, handlers, stackHelpers +} + +// isStackMethod reports whether fn is a method on *Stack. +func isStackMethod(fn *ast.FuncDecl) bool { + if fn.Recv == nil || len(fn.Recv.List) != 1 { + return false + } + star, ok := fn.Recv.List[0].Type.(*ast.StarExpr) + if !ok { + return false + } + id, ok := star.X.(*ast.Ident) + return ok && id.Name == "Stack" +} + +// hasInlineMarker reports whether fn is tagged //gen:inline, which marks a stack +// helper for splicing into the generated dispatch. +func hasInlineMarker(fn *ast.FuncDecl) bool { + if fn.Doc == nil { + return false + } + for _, c := range fn.Doc.List { + if c.Text == "//gen:inline" { + return true + } + } + return false } var returnRe = regexp.MustCompile(`^(\s*)return\s+([^,]+),\s*(.+)$`) @@ -164,7 +199,7 @@ func (g *generator) inlineBody(handler string) string { if fn == nil { fatalf("no handler %q to inline", handler) } - return g.rewriteSplicedBody(g.renderAst(fn.Body.List)) + return g.rewriteSplicedBody(g.expandStackHelpers(fn.Body.List, nil)) } // inlineFactoryBody splices the body of the executionFunc closure that a make* @@ -179,20 +214,16 @@ func (g *generator) inlineFactoryBody(factory string, args ...int) string { fatalf("no factory %q to inline", factory) } lit := factoryClosure(factory, fn) - src := g.renderAst(lit.Body.List) - var names []string - for _, f := range fn.Type.Params.List { - for _, nm := range f.Names { - names = append(names, nm.Name) - } - } + // Bind the factory parameters to the per-opcode constants, then inline. + names := paramNames(fn) if len(names) != len(args) { fatalf("factory %q takes %d params, got %d args", factory, len(names), len(args)) } + params := map[string]int{} for i, nm := range names { - src = regexp.MustCompile(`\b`+nm+`\b`).ReplaceAllString(src, fmt.Sprint(args[i])) + params[nm] = args[i] } - return g.rewriteSplicedBody(src) + return g.rewriteSplicedBody(g.expandStackHelpers(lit.Body.List, params)) } // factoryClosure returns the executionFunc literal that a make* factory's body @@ -230,7 +261,8 @@ func (g *generator) renderAst(stmts []ast.Stmt) string { // rewriteSplicedBody rewrites a printed handler body so it runs inside the // dispatch loop: the `*pc` dereference becomes the loop's `pc` local, and each // `return r0, r1` becomes loop control flow. Success (r1 == nil) advances pc -// and continues, an error sets err and breaks. Stack helpers are then expanded. +// and continues, an error sets err and breaks. (Stack helpers were already +// inlined by expandStackHelpers before the body was printed.) func (g *generator) rewriteSplicedBody(src string) string { src = strings.ReplaceAll(src, "*pc", "pc") @@ -258,40 +290,200 @@ func (g *generator) rewriteSplicedBody(src string) string { } out.WriteString(line + "\n") } - return expandStackHelpers(out.String()) + return out.String() } -// The compiler shrinks its inlining budget into very large functions, and -// execUntraced is far past the size threshold. The cheap stack helpers -// (pop1, len, peek, drop, back) still inline there, but get and the -// pop1Peek1 family turn into real calls, which a snailtracer profile put at -// over a tenth of the run. The generator splices those helper bodies in -// textually wherever they appear in statement position, verbatim from -// stack.go. The handler sources are kept to that statement form (no var blocks, -// no method chaining on a helper call) so each helper is one statement-anchored -// regex. -var ( - pop1Peek1Re = regexp.MustCompile(`(?m)^(\s*)(\w+), (\w+) := scope\.Stack\.pop1Peek1\(\)$`) - pop2Peek1Re = regexp.MustCompile(`(?m)^(\s*)(\w+), (\w+), (\w+) := scope\.Stack\.pop2Peek1\(\)$`) - pop2Re = regexp.MustCompile(`(?m)^(\s*)(\w+), (\w+) := scope\.Stack\.pop2\(\)$`) - getAssignRe = regexp.MustCompile(`(?m)^(\s*)(\w+) := scope\.Stack\.get\(\)$`) - dupRe = regexp.MustCompile(`(?m)^(\s*)scope\.Stack\.dup\((\d+)\)$`) -) +// The helpers spliced inline are tagged //gen:inline in stack.go and collected +// into g.stackHelpers. They cost more than the compiler will inline into a +// function the size of execUntraced, where a snailtracer profile put the calls +// at over a tenth of the run. The cheap helpers (pop1, len, peek, drop, back) +// inline on their own and stay as calls. Each tagged helper is inlined from its +// stack.go body (expandStackHelpers), so the inlined code follows the one definition. -func expandStackHelpers(src string) string { - src = pop1Peek1Re.ReplaceAllString(src, - "${1}stack.inner.top--\n${1}stack.size--\n${1}$2 := &stack.inner.data[stack.inner.top]\n${1}$3 := &stack.inner.data[stack.inner.top-1]") - src = pop2Peek1Re.ReplaceAllString(src, - "${1}stack.inner.top -= 2\n${1}stack.size -= 2\n${1}$2 := &stack.inner.data[stack.inner.top+1]\n${1}$3 := &stack.inner.data[stack.inner.top]\n${1}$4 := &stack.inner.data[stack.inner.top-1]") - src = pop2Re.ReplaceAllString(src, - "${1}stack.inner.top -= 2\n${1}stack.size -= 2\n${1}$2 := &stack.inner.data[stack.inner.top+1]\n${1}$3 := &stack.inner.data[stack.inner.top]") - src = getAssignRe.ReplaceAllString(src, - "${1}$2 := &stack.inner.data[stack.inner.top]\n${1}stack.inner.top++\n${1}stack.size++") - src = dupRe.ReplaceAllString(src, - "${1}stack.inner.data[stack.bottom+stack.size] = stack.inner.data[stack.bottom+stack.size-$2]\n${1}stack.size++\n${1}stack.inner.top++") +// stackCall is a matched call to a tagged helper. +type stackCall struct { + helper string // helper method name + lhs []ast.Expr // assignment targets, nil for a void call like dup + tok token.Token // the assignment token, := or = + args []ast.Expr // call arguments (only dup has one) +} + +// matchStackHelper matches a statement that is a single must-expand helper call, +// in one of the two normalized forms: an assignment `lhs... := scope.Stack.H(args)` +// or a bare `scope.Stack.H(args)`. +func (g *generator) matchStackHelper(stmt ast.Stmt) (stackCall, bool) { + switch s := stmt.(type) { + case *ast.AssignStmt: + if len(s.Rhs) == 1 { + if h, args, ok := g.stackHelperCall(s.Rhs[0]); ok { + return stackCall{helper: h, lhs: s.Lhs, tok: s.Tok, args: args}, true + } + } + case *ast.ExprStmt: + if h, args, ok := g.stackHelperCall(s.X); ok { + return stackCall{helper: h, args: args}, true + } + } + return stackCall{}, false +} + +// stackHelperCall unwraps scope.Stack.H(args) where H is a must-expand helper. +func (g *generator) stackHelperCall(e ast.Expr) (helper string, args []ast.Expr, ok bool) { + call, isCall := e.(*ast.CallExpr) + if !isCall { + return "", nil, false + } + sel, isSel := call.Fun.(*ast.SelectorExpr) // .H + if !isSel || g.stackHelpers[sel.Sel.Name] == nil || !isStackExpr(sel.X) { + return "", nil, false + } + return sel.Sel.Name, call.Args, true +} + +// isStackExpr reports whether e is the stack receiver: the `stack` local or +// scope.Stack. +func isStackExpr(e ast.Expr) bool { + switch x := e.(type) { + case *ast.Ident: + return x.Name == "stack" + case *ast.SelectorExpr: + return x.Sel.Name == "Stack" + } + return false +} + +// expandStackHelpers renders a handler body to source, inlining every must-expand +// helper call and printing other statements unchanged. params maps the factory +// parameters (makePush/makeDup) to their per-opcode constants. Helper calls +// appear only at statement top-level here; a nested one would be left as a real +// call and caught by the inlining guard test. +func (g *generator) expandStackHelpers(stmts []ast.Stmt, params map[string]int) string { + var out strings.Builder + for _, stmt := range stmts { + if call, ok := g.matchStackHelper(stmt); ok { + out.WriteString(g.inlineStackHelper(call, params)) + } else { + out.WriteString(substParams(g.renderAst([]ast.Stmt{stmt}), params)) + } + } + return out.String() +} + +// substParams replaces each factory parameter with its constant. It runs only +// on printed non-helper statements and on helper arguments, never on a helper +// expansion, so it cannot touch a field like stack.size. The parameter names do +// not textually overlap, so map order does not affect the result. +func substParams(src string, params map[string]int) string { + for name, val := range params { + src = regexp.MustCompile(`\b`+name+`\b`).ReplaceAllString(src, fmt.Sprint(val)) + } return src } +// inlineStackHelper expands one helper call to its stack.go body. The single +// rule: the helper is straight-line statements then an optional final return +// whose result count matches the call's targets. Anything else is not in +// inlinable form and is a hard error (the shape post-condition). The receiver +// maps to the loop's `stack` local and each parameter to its call argument. +func (g *generator) inlineStackHelper(call stackCall, params map[string]int) string { + fn := g.stackHelpers[call.helper] + if fn == nil { + fatalf("no stack helper %q to inline", call.helper) + } + // Peel an optional trailing return off the body. + body := fn.Body.List + var ret *ast.ReturnStmt + if n := len(body); n > 0 { + if r, isRet := body[n-1].(*ast.ReturnStmt); isRet { + ret, body = r, body[:n-1] + } + } + results := 0 + if ret != nil { + results = len(ret.Results) + } + if len(call.lhs) != results { + fatalf("stack helper %q returns %d values, call assigns %d", call.helper, results, len(call.lhs)) + } + // Map the receiver to the loop local and each parameter to its argument. + names := paramNames(fn) + if len(names) != len(call.args) { + fatalf("stack helper %q takes %d params, call passes %d", call.helper, len(names), len(call.args)) + } + subst := map[string]string{recvName(fn): "stack"} + for i, name := range names { + subst[name] = substParams(renderInlineExpr(call.args[i], nil), params) + } + // The leading bookkeeping statements, then bind each return expression to + // its assignment target. + var out strings.Builder + for _, stmt := range body { + out.WriteString(renderInlineStmt(stmt, subst) + "\n") + } + for i, lhs := range call.lhs { + out.WriteString(renderInlineExpr(lhs, nil) + " " + call.tok.String() + " " + renderInlineExpr(ret.Results[i], subst) + "\n") + } + return out.String() +} + +// recvName returns a method's receiver name (e.g. "s"). +func recvName(fn *ast.FuncDecl) string { + if names := fn.Recv.List[0].Names; len(names) > 0 { + return names[0].Name + } + return "" +} + +// paramNames returns a function's parameter names, in order. +func paramNames(fn *ast.FuncDecl) []string { + var names []string + for _, f := range fn.Type.Params.List { + for _, nm := range f.Names { + names = append(names, nm.Name) + } + } + return names +} + +// renderInlineStmt prints one helper-body statement with subst applied. Only the +// statement shapes the helpers use are handled; any other is not inlinable. +func renderInlineStmt(stmt ast.Stmt, subst map[string]string) string { + switch s := stmt.(type) { + case *ast.IncDecStmt: // s.inner.top++ + return renderInlineExpr(s.X, subst) + s.Tok.String() + case *ast.AssignStmt: // s.size -= 2 or data[x] = data[y] + if len(s.Lhs) == 1 && len(s.Rhs) == 1 { + return renderInlineExpr(s.Lhs[0], subst) + " " + s.Tok.String() + " " + renderInlineExpr(s.Rhs[0], subst) + } + } + fatalf("inline: unsupported statement %T in stack helper", stmt) + return "" +} + +// renderInlineExpr prints one helper-body expression, substituting any +// identifier found in subst. Only the shapes the helpers use are handled. +func renderInlineExpr(expr ast.Expr, subst map[string]string) string { + switch e := expr.(type) { + case *ast.Ident: + if r, ok := subst[e.Name]; ok { + return r + } + return e.Name + case *ast.BasicLit: + return e.Value + case *ast.SelectorExpr: // x.field + return renderInlineExpr(e.X, subst) + "." + e.Sel.Name + case *ast.IndexExpr: // x[i] + return renderInlineExpr(e.X, subst) + "[" + renderInlineExpr(e.Index, subst) + "]" + case *ast.BinaryExpr: // x op y + return renderInlineExpr(e.X, subst) + " " + e.Op.String() + " " + renderInlineExpr(e.Y, subst) + case *ast.UnaryExpr: // &x + return e.Op.String() + renderInlineExpr(e.X, subst) + } + fatalf("inline: unsupported expression %T in stack helper", expr) + return "" +} + // --------------------------------------------------------------------------- // Metadata derivation (from the per-fork tables, via vm.GenForks) // --------------------------------------------------------------------------- @@ -632,8 +824,8 @@ func main() { } vmDir := filepath.Dir(filepath.Dir(self)) // .../core/vm/gen -> .../core/vm - fset, handlers := parseHandlers(vmDir) - g := &generator{fset: fset, handlers: handlers, buf: new(bytes.Buffer)} + fset, handlers, stackHelpers := parseHandlers(vmDir) + g := &generator{fset: fset, handlers: handlers, stackHelpers: stackHelpers, buf: new(bytes.Buffer)} g.deriveMeta(vm.GenForks()) g.emitFile() diff --git a/core/vm/interp_gen.go b/core/vm/interp_gen.go index 94fe339df5..0e7e324dec 100644 --- a/core/vm/interp_gen.go +++ b/core/vm/interp_gen.go @@ -705,9 +705,9 @@ mainLoop: return nil, ErrOutOfGas } contract.Gas.RegularGas -= 2 - elem := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + elem := &stack.inner.data[stack.inner.top-1] elem.SetUint64(pc) pc++ continue mainLoop @@ -720,9 +720,9 @@ mainLoop: return nil, ErrOutOfGas } contract.Gas.RegularGas -= 2 - elem := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + elem := &stack.inner.data[stack.inner.top-1] elem.SetUint64(uint64(scope.Memory.Len())) pc++ continue mainLoop @@ -744,9 +744,9 @@ mainLoop: return nil, ErrOutOfGas } contract.Gas.RegularGas -= 2 - elem := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + elem := &stack.inner.data[stack.inner.top-1] elem.Clear() pc++ continue mainLoop @@ -771,9 +771,9 @@ mainLoop: continue mainLoop } codeLen := uint64(len(scope.Contract.Code)) - elem := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + elem := &stack.inner.data[stack.inner.top-1] pc += 1 if pc < codeLen { elem.SetUint64(uint64(scope.Contract.Code[pc])) @@ -800,9 +800,9 @@ mainLoop: continue mainLoop } codeLen := uint64(len(scope.Contract.Code)) - elem := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + elem := &stack.inner.data[stack.inner.top-1] if pc+2 < codeLen { elem.SetBytes2(scope.Contract.Code[pc+1 : pc+3]) } else if pc+1 < codeLen { @@ -835,9 +835,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+3) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 3 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -867,9 +867,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+4) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 4 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -899,9 +899,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+5) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 5 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -931,9 +931,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+6) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 6 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -963,9 +963,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+7) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 7 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -995,9 +995,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+8) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 8 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -1027,9 +1027,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+9) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 9 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -1059,9 +1059,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+10) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 10 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -1091,9 +1091,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+11) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 11 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -1123,9 +1123,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+12) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 12 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -1155,9 +1155,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+13) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 13 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -1187,9 +1187,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+14) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 14 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -1219,9 +1219,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+15) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 15 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -1251,9 +1251,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+16) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 16 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -1283,9 +1283,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+17) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 17 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -1315,9 +1315,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+18) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 18 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -1347,9 +1347,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+19) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 19 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -1379,9 +1379,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+20) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 20 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -1411,9 +1411,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+21) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 21 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -1443,9 +1443,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+22) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 22 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -1475,9 +1475,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+23) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 23 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -1507,9 +1507,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+24) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 24 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -1539,9 +1539,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+25) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 25 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -1571,9 +1571,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+26) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 26 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -1603,9 +1603,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+27) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 27 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -1635,9 +1635,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+28) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 28 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -1667,9 +1667,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+29) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 29 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -1699,9 +1699,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+30) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 30 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -1731,9 +1731,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+31) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 31 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) @@ -1763,9 +1763,9 @@ mainLoop: start = min(codeLen, int(pc+1)) end = min(codeLen, start+32) ) - a := &stack.inner.data[stack.inner.top] stack.inner.top++ stack.size++ + a := &stack.inner.data[stack.inner.top-1] a.SetBytes(scope.Contract.Code[start:end]) if missing := 32 - (end - start); missing > 0 { a.Lsh(a, uint(8*missing)) diff --git a/core/vm/interp_gen_test.go b/core/vm/interp_gen_test.go index 26807ed84e..119faba910 100644 --- a/core/vm/interp_gen_test.go +++ b/core/vm/interp_gen_test.go @@ -403,50 +403,61 @@ func FuzzInterpreterDiff(f *testing.F) { }) } -// expandedHelpers are the costly *Stack helpers the generator splices inline -// (see expandStackHelpers in core/vm/gen) instead of leaving as calls, because -// they exceed the inline budget for a function as large as execUntraced. After -// generation none should remain as a call in interp_gen.go. If a change to the -// handler source or to stack.go alters how one is written, the expansion regex -// silently stops matching and the helper survives as a real call: correct, but -// it drops the inlining the fast path exists for. This is the generator's old -// post-condition, moved here so both halves of the invariant live together. -var expandedHelpers = []string{"get", "dup", "pop2", "pop1Peek1", "pop2Peek1"} +// markedHelpers parses stack.go and returns the *Stack helpers tagged +// //gen:inline. That tag is the single source of truth, shared with the +// generator (core/vm/gen), for which helpers are spliced into the dispatch. +func markedHelpers(t *testing.T) map[string]bool { + t.Helper() + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, "stack.go", nil, parser.ParseComments) + if err != nil { + t.Fatalf("parsing stack.go: %v", err) + } + marked := map[string]bool{} + for _, decl := range f.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Doc == nil { + continue + } + for _, c := range fn.Doc.List { + if c.Text == "//gen:inline" { + marked[fn.Name.Name] = true + } + } + } + if len(marked) == 0 { + t.Fatal("found no //gen:inline helpers in stack.go") + } + return marked +} // TestGeneratedFastPathHelpersExpanded asserts the generator spliced every -// costly stack helper inline, so none survives as a call in interp_gen.go. It -// is the expand-side counterpart to TestGeneratedFastPathHelpersInlined: together -// they hold the one invariant that the fast path makes no real stack-helper -// call, the costly ones by splicing, the cheap ones by compiler inlining. +// //gen:inline helper inline, so none survives as a real call in interp_gen.go. +// Those helpers exceed the compiler's inline budget for a function as large as +// execUntraced, so a missed splice would silently drop the inlining the fast +// path exists for. It is the expand-side counterpart to +// TestGeneratedFastPathHelpersInlined: together they hold the one invariant that +// the fast path makes no real stack-helper call, the costly ones by splicing, +// the cheap ones by compiler inlining. func TestGeneratedFastPathHelpersExpanded(t *testing.T) { calls := countStackCalls(t, "interp_gen.go") - for _, h := range expandedHelpers { + for h := range markedHelpers(t) { if n := calls[h]; n != 0 { - t.Errorf("(*Stack).%s: %d residual call(s) in interp_gen.go, expected 0.\n"+ - "The generator no longer splices it inline, so it leaked through as a real call.\n"+ - "Fix the matching regex in expandStackHelpers (core/vm/gen).", h, n) + t.Errorf("(*Stack).%s is //gen:inline but has %d residual call(s) in interp_gen.go, expected 0.\n"+ + "The generator did not splice it. Check it is still in inlinable shape (core/vm/gen).", h, n) } } } -// mustInlineHelpers are the cheap *Stack helpers the generated fast path calls -// directly and trusts the compiler to inline into execUntraced. Unlike get, -// dup, pop2, pop1Peek1 and pop2Peek1, which are too costly to inline into a -// function that large and so are spliced in textually by the generator (see -// expandStackHelpers in core/vm/gen, checked by TestGeneratedFastPathHelpersExpanded), -// these stay as plain calls. They inline today, most with comfortable margin, but pop1 sits -// at cost 18 against Go's big-function budget of 20. A toolchain that re-scores -// inline cost, or an extra branch in one of these bodies, could silently stop -// the inlining and quietly slow the interpreter. This is the set we refuse to -// let regress in silence. back is absent because it has no call site here. -var mustInlineHelpers = []string{"len", "pop1", "peek", "drop"} - // TestGeneratedFastPathHelpersInlined recompiles this package with the -// compiler's inlining diagnostics on and fails if any call to a mustInlineHelper -// in interp_gen.go was not inlined. It is the inlining-side counterpart to -// TestGeneratedFastPathHelpersExpanded, which checks the expensive helpers were -// spliced away. Together they keep both halves of the fast path honest: the -// costly helpers stay spliced, the cheap ones stay inlined. +// compiler's inlining diagnostics on and fails if any *Stack helper call that +// survives into interp_gen.go was not inlined. Every survivor must be a cheap +// helper (len, pop1, peek, drop) the compiler inlines into execUntraced; the +// //gen:inline helpers are spliced away and owned by the Expanded test. The +// cheap ones inline today with margin except pop1, at cost 18 against Go's +// big-function budget of 20. A toolchain that re-scores inline cost, or an extra +// branch in one of these bodies, could silently stop the inlining and slow the +// interpreter, so this turns that into a red build. func TestGeneratedFastPathHelpersInlined(t *testing.T) { if testing.Short() { t.Skip("skipping inlining check (recompiles the package) in -short mode") @@ -464,18 +475,22 @@ func TestGeneratedFastPathHelpersInlined(t *testing.T) { t.Fatalf("captured no interp_gen.go diagnostics, the -m build produced nothing to check:\n%s", diag) } - calls := countStackCalls(t, "interp_gen.go") - for _, h := range mustInlineHelpers { + // Every surviving stack-helper call (i.e. not a //gen:inline target) must be + // inlined by the compiler. + marked := markedHelpers(t) + for h, n := range countStackCalls(t, "interp_gen.go") { + if marked[h] { + continue // spliced away, owned by TestGeneratedFastPathHelpersExpanded + } inlinedRe := regexp.MustCompile(`interp_gen\.go.*inlining call to \(\*Stack\)\.` + regexp.QuoteMeta(h) + `\b`) inlined := len(inlinedRe.FindAllString(diag, -1)) - if calls[h] != inlined { + if inlined != n { t.Errorf("(*Stack).%s: %d call site(s) in interp_gen.go, %d inlined into execUntraced.\n"+ "The compiler stopped inlining it, so the fast path now pays a real call. Shrink the\n"+ - "body to fit the inline budget, or promote it to a spliced helper (add an expansion to\n"+ - "expandStackHelpers in core/vm/gen and move it to expandedHelpers here).", h, calls[h], inlined) + "body to fit the inline budget, or tag it //gen:inline in stack.go to splice it instead.", h, n, inlined) continue } - t.Logf("(*Stack).%s: %d/%d call sites inlined", h, inlined, calls[h]) + t.Logf("(*Stack).%s: %d/%d call sites inlined", h, inlined, n) } } diff --git a/core/vm/stack.go b/core/vm/stack.go index 9b208ff07a..342526c397 100644 --- a/core/vm/stack.go +++ b/core/vm/stack.go @@ -104,6 +104,8 @@ func (s *Stack) push(d *uint256.Int) { // get returns a pointer to a newly created element // on top of the stack +// +//gen:inline func (s *Stack) get() *uint256.Int { s.inner.top++ s.size++ @@ -136,6 +138,8 @@ func (s *Stack) pop1() *uint256.Int { // pop2 removes the top two elements and returns pointers to them. The // pointers stay valid only until the next push or sub call. +// +//gen:inline func (s *Stack) pop2() (top, second *uint256.Int) { s.inner.top -= 2 s.size -= 2 @@ -161,6 +165,8 @@ func (s *Stack) pop4() (top, second, third, fourth *uint256.Int) { // pop1Peek1 removes the top element and returns pointers to it and to the new // top, the usual operand and write target of a binary operation. The first // pointer stays valid only until the next push or sub call. +// +//gen:inline func (s *Stack) pop1Peek1() (top, rest *uint256.Int) { s.inner.top-- s.size-- @@ -170,6 +176,8 @@ func (s *Stack) pop1Peek1() (top, rest *uint256.Int) { // pop2Peek1 removes the top two elements and returns pointers to them and to // the new top, for three operand operations. The first two pointers stay // valid only until the next push or sub call. +// +//gen:inline func (s *Stack) pop2Peek1() (top, second, rest *uint256.Int) { s.inner.top -= 2 s.size -= 2 @@ -225,6 +233,7 @@ func (s *Stack) swap16() { s.inner.data[s.bottom+s.size-17], s.inner.data[s.bottom+s.size-1] = s.inner.data[s.bottom+s.size-1], s.inner.data[s.bottom+s.size-17] } +//gen:inline func (s *Stack) dup(n int) { s.inner.data[s.bottom+s.size] = s.inner.data[s.bottom+s.size-n] s.size++