Multi-return functions, global constants, and op-cost-aware function inlining

.cspell.json

@@ -76,6 +76,7 @@
         "gotchas",
         "greaterthan",
         "greaterthanorequal",
+        "groth",
         "hardcodes",
         "hash",
         "hashoutputs",

docs/function-call-convention.md

@@ -0,0 +1,88 @@
+# The user-function calling convention and its op-cost
+
+This note documents the argument-staging convention for user-defined function calls
+(`OP_DEFINE`/`OP_INVOKE`), why the original upstream convention caused a measurable op-cost
+regression on call-dense contracts, and the fix (`d0409c2`). It is written against the
+`feat/multi-returns` branch.
+
+## The regression
+
+After porting the zk-verifier contracts from the old `feat/library-support` fork to this branch,
+every byte-scored benchmark entry improved, but the BN254 op-bound multi-input families regressed
++1.3–2.3%. Per-stage decomposition localised the loss to *unrolled, call-dense* bodies (the lazy
+tower's fused-miller and final-exp stages: thousands of small multi-return calls per chunk), while
+*loop-shaped* stages improved. Constant hoisting and inlining were ruled out by direct measurement;
+disassembly of the same Miller chunk under both compilers showed the new one emitting smaller
+bytecode but executing ~5% more instructions.
+
+## The cause
+
+**The argument-staging convention at `OP_INVOKE` call sites.** The compiler expected the *last*
+parameter on top of the stack at function entry ("like builtin functions"). Arguments are staged by
+bringing each one to the top of the stack in turn — so staging them left-to-right leaves the last
+argument on top, which sounds like a match. But the staging ops themselves are what cost: for the
+overwhelmingly common call shape — variables passed in declaration order, e.g. `fp2Sub(a0, a1, b0,
+b1)` over locals laid out `[a0, a1, b0, b1]` (first on top) — building the *reversed* layout costs
+real reversal instructions at **every call site**:
+
+```
+convention: last param on top          convention: first param on top
+--------------------------------      --------------------------------
+        OP_SWAP OP_0 OP_INVOKE                OP_0 OP_INVOKE
+OP_2DUP OP_SWAP OP_0 OP_INVOKE        OP_2DUP OP_0 OP_INVOKE
+```
+
+(2-argument call, measured; with 4–6 arguments the reversal grows into `OP_SWAP OP_2SWAP OP_SWAP`
+chains.) The old fork compiled bodies against a **first-parameter-on-top** entry layout and staged
+arguments **right-to-left**: each argument is still brought to the top in turn, but in reverse
+order, so for declaration-order variable arguments the emitted `<n> OP_ROLL` pairs cancel to
+nothing in the peephole optimiser. A typical call costs **zero staging instructions**.
+
+The overhead was ~1–3 executed instructions per call depending on arity. On the BN254 lazy tower
+(~12–14k executed calls per proof) that multiplied to ~+2.9M op-cost on the residue pipeline and
+~+5M on groth16-chunked — and for op-bound contracts, whose unlocking scripts are zero-padded until
+`(41 + length) × 800` covers the op budget, every 800 op-cost is one more mandatory padding byte.
+
+## The fix
+
+`GenerateTargetTraversal` now:
+
+1. **Seeds function bodies with the first parameter on top** (`compileGlobalFunctionBody` visits
+   parameters in declaration order; `visitParameter` pushes to the stack bottom).
+2. **Stages user-function call arguments right-to-left** (`stageUserFunctionArguments`), so the
+   first argument lands on top, matching the body layout. Built-in functions keep natural
+   left-to-right evaluation.
+3. **Guards ROLL under reversed emission**: evaluating arguments right-to-left breaks the textual
+   final-use order that the `opRolls` analysis assumes, so within a call's argument tree a variable
+   may only be ROLLed if it appears exactly once across the whole (possibly nested) tree
+   (`ArgIdentifierCounter`; see the `isOpRoll` guard). Everything else stays a PICK; leftover
+   originals are cleaned up once at the end of the spend path.
+
+Output is byte-identical to the old fork on representative multi-return call patterns (2-arg and
+4-arg, including argument reuse and nested calls).
+
+## Static size vs executed cost
+
+The two conventions sit at different points on a per-call vs per-spend spectrum, so **static
+bytecode size can grow while executed op-cost drops**:
+
+- reversal staging (removed) was paid in bytes *and* executed instructions at **every call site**;
+- PICK-instead-of-ROLL residue and end-of-spend cleanup (`OP_NIP`/`OP_2DROP` chains) are paid in
+  bytes but executed **once per spend**.
+
+Measured on real contracts: loop-shaped `vkx.cash` improves on both axes (−8 bytes, −14 static
+ops); unrolled `miller_00.cash` grows +111 static bytes while executed instructions drop (the old
+fork's equivalent chunk was +425 bytes static and ~5% cheaper dynamically). Op-bound contracts
+price executed cost, so this is the right default trade. If the small static give-back ever matters
+for a byte-scored, call-dense contract, per-call staging could be resurrected behind
+`optimizeFor: 'size'` — deliberately not done now to avoid conventions diverging without a
+demonstrated need.
+
+## Interaction with the rest of the pipeline
+
+- **Inlining** is unaffected: an inlined body is compiled against the same staged-arguments layout
+  and spliced where the arguments sit.
+- **Multi-return values** still leave the last-declared value on top; destructuring is unchanged.
+- **Debug frames** pin the body bytecode, which changed shape under the new layout — the pinned
+  generation and BitAuth-script fixtures were updated accordingly.
+- **Recursion** still falls back to `OP_DEFINE`/`OP_INVOKE` via the invoked-functions guard.

packages/cashc/src/Errors.ts

@@ -3,6 +3,7 @@ import {
   IdentifierNode,
   ImportNode,
   FunctionDefinitionNode,
+  ConstantDefinitionNode,
   VariableDefinitionNode,
   ParameterNode,
   Node,
@@ -22,6 +23,7 @@ import {
   ExpressionNode,
   SliceNode,
   IntLiteralNode,
+  TupleAssignmentNode,
 } from './ast/AST.js';
 import { Symbol, SymbolType } from './ast/SymbolTable.js';
 import { Location } from './ast/Location.js';
@@ -84,6 +86,32 @@ export class FunctionRedefinitionError extends RedefinitionError {
   }
 }
 
+export class ConstantDefinitionError extends CashScriptError {
+  constructor(
+    public node: ConstantDefinitionNode,
+    message: string,
+  ) {
+    super(node, message);
+  }
+}
+
+export class ConstantRedefinitionError extends RedefinitionError {
+  constructor(
+    public node: ConstantDefinitionNode,
+  ) {
+    super(node, `Redefinition of constant ${node.name}`);
+  }
+}
+
+export class ConstantNameCollisionError extends CashScriptError {
+  constructor(
+    node: Node,
+    name: string,
+  ) {
+    super(node, `Identifier '${name}' collides with a global constant of the same name`);
+  }
+}
+
 export class MissingContractError extends Error {
   constructor() {
     super('Source file does not contain a contract definition');
@@ -198,6 +226,35 @@ export class ReturnTypeError extends TypeError {
   }
 }
 
+export class ReturnCountError extends CashScriptError {
+  constructor(
+    node: Node,
+    actual: number,
+    expected: number,
+  ) {
+    super(node, `Function returns ${actual} value(s) but ${expected} were declared`);
+  }
+}
+
+export class TupleArityError extends CashScriptError {
+  constructor(
+    node: Node,
+    targetCount: number,
+    valueCount: number,
+  ) {
+    super(node, `Cannot destructure ${valueCount} value(s) into ${targetCount} variable(s)`);
+  }
+}
+
+export class MultiReturnDestructureError extends CashScriptError {
+  constructor(
+    node: FunctionCallNode,
+    count: number,
+  ) {
+    super(node, `Function '${node.identifier.name}' returns ${count} values and must be destructured into ${count} variables`);
+  }
+}
+
 export class InvalidParameterTypeError extends TypeError {
   constructor(
     node: FunctionCallNode | RequireNode | InstantiationNode,
@@ -280,6 +337,23 @@ export class TupleAssignmentError extends CashScriptError {
   }
 }
 
+export class DuplicateTupleTargetError extends CashScriptError {
+  constructor(
+    node: TupleAssignmentNode,
+    name: string,
+  ) {
+    super(node, `Duplicate target '${name}' in tuple destructuring`);
+  }
+}
+
+export class TupleTargetOrderError extends CashScriptError {
+  constructor(
+    node: TupleAssignmentNode,
+  ) {
+    super(node, 'Declaration targets must come before all reassignment targets in a tuple destructuring');
+  }
+}
+
 export class ConstantConditionError extends CashScriptError {
   constructor(
     node: BranchNode | RequireNode,
@@ -297,6 +371,8 @@ export class ConstantModificationError extends CashScriptError {
   }
 }
 
+export class InvalidModifierError extends CashScriptError {}
+
 export class ArrayElementError extends CashScriptError {
   constructor(
     node: ArrayNode,

packages/cashc/src/ast/AST.ts

@@ -35,6 +35,7 @@ export class SourceFileNode extends Node {
     public functions: FunctionDefinitionNode[] = [],
     public imports: ImportNode[] = [],
     public pragmas: string[] = [],
+    public constants: ConstantDefinitionNode[] = [],
   ) {
     super();
   }
@@ -44,6 +45,23 @@ export class SourceFileNode extends Node {
   }
 }
 
+// A compile-time constant (file top level). Its initializer is folded to a literal and inlined at
+// every use site by the constant-inlining pass, after which this node is discarded — it is never
+// visited by an AstVisitor.
+export class ConstantDefinitionNode extends Node implements Named, Typed {
+  constructor(
+    public type: Type,
+    public name: string,
+    public expression: ExpressionNode,
+  ) {
+    super();
+  }
+
+  accept<T>(): T {
+    throw new Error('ConstantDefinitionNode must be inlined before AST traversal');
+  }
+}
+
 export class ImportNode extends Node {
   constructor(
     public path: string,
@@ -85,7 +103,7 @@ export class FunctionDefinitionNode extends Node implements Named {
     public name: string,
     public parameters: ParameterNode[],
     public body: BlockNode,
-    public returnType?: Type,
+    public returnTypes?: Type[],
   ) {
     super();
   }
@@ -99,6 +117,9 @@ export class ParameterNode extends Node implements Named, Typed {
   constructor(
     public type: Type,
     public name: string,
+    // Declaration modifiers (e.g. `unused`, which exempts the parameter from the unused-variable
+    // check — useful for padding bytes that buy a larger compute budget without being referenced).
+    public modifiers: string[] = [],
   ) {
     super();
   }
@@ -127,11 +148,20 @@ export class VariableDefinitionNode extends NonControlStatementNode implements N
   }
 }
 
+export interface TupleTarget {
+  name: string;
+  // For a fresh-declaration target (`int x`) this is the declared type. For a reassignment target
+  // (`x`, no type) it is undefined at parse time and filled in from the existing variable's symbol
+  // during SymbolTableTraversal.
+  type?: Type;
+  // True for a reassignment of an already-declared variable (no `typeName` in source).
+  isReassignment?: boolean;
+}
+
 export class TupleAssignmentNode extends NonControlStatementNode {
   constructor(
-    // TODO: Use an IdentifierNode instead of a custom type
-    public left: { name: string, type: Type },
-    public right: { name: string, type: Type },
+    // TODO: Use IdentifierNodes instead of a custom type
+    public targets: TupleTarget[],
     public tuple: ExpressionNode,
   ) {
     super();
@@ -211,7 +241,7 @@ export class FunctionCallStatementNode extends NonControlStatementNode {
 
 export class ReturnNode extends NonControlStatementNode {
   constructor(
-    public expression: ExpressionNode,
+    public expressions: ExpressionNode[],
   ) {
     super();
   }