Interpreter: Make function calls and basic instructions working #1753

RishabhRD · 2025-10-30T10:12:27Z

Design Changes

Introduced notion of function parameters as it was part of IR.
Unify the memory representation of stack and heap objects.
Instructions results in InstructionResult type instead of Any upon execution.

Unified Memory Representation

Pointer instructions require conversion from pointer to address and vice-versa.
For enabling this mechanism, there should be some way to encode a memory address as pointer.
With UUID used in stack addressing, this mechanism is not possible without overhead of huge storage as UUID itself is 128 bits and pointer on any machine can be maximum 64 bits. And for supporting any generic target ABI, this is not the way.
Also different address for stack and heap objects would waste a discriminator bit too.
Hence, we need to unify the way to address stack and heap objects.

Design

Stack is also a memory allocation now that just stores local variables.
Stack now has fixed memory. Stack gets created on initialization of interpreter.
StackFrame is now just responsible for registers, return address and parameters. There are classic techiniques which can also make returnAddress and parameters part of stack. I am not sure if we want that for now.
Because stack is also a memory allocation now, addresses for heap objects and stack objects are of same type and will be useful for implementing pointer operations.

Instructions implemented:

Access
AllocStack
Branch
Call
CondBranch
DeallocStack
Load
MemoryCopy
Store [Partial support for only integer types]
SubfieldView
Load

Instructions UN-implemented

ConstantString

…tion_call

Sources/Interpreter/TargetABI.swift

Sources/Interpreter/Interpreter.swift

dabrahams · 2025-10-31T20:09:22Z

Sources/Interpreter/Interpreter.swift

+
+}
+
+struct ObjectAddress {


Should this be called “Pointer?” I don't think “Object” is adding anything.

There are instructions like AddressToPointer, thus I didn't pick the name Pointer. But yeah, "object" can be removed from name.

Sources/Interpreter/Interpreter.swift

dabrahams

I've only just started with the review, but I must say it's hard to evaluate these diffs, because entities like StackFrame do not show up in the same place.

RishabhRD · 2025-10-31T21:01:20Z

I've only just started with the review, but I must say it's hard to evaluate these diffs, because entities like StackFrame do not show up in the same place.

Sorry! I accidentally moved stackframe down, fixed that. I should have observed that while raising PR itself.

Sources/Interpreter/Interpreter.swift

dabrahams · 2025-11-06T22:18:29Z

Sources/Interpreter/UntypedBuiltinValue.swift

+  }
+
+  /// UInt8 value, if present.
+  public var uint8: UInt8? { size == 1 ? UInt8(truncatingIfNeeded: asUInt128) : nil }


Suggested change

public var uint8: UInt8? { size == 1 ? UInt8(truncatingIfNeeded: asUInt128) : nil }

public var i8: UInt8? { size == 1 ? UInt8(truncatingIfNeeded: asUInt128) : nil }

Because that's what we call it in instructions

dabrahams · 2025-11-06T22:33:48Z

Sources/Interpreter/UntypedBuiltinValue.swift

+import IR
+
+/// The value of a `Builtin` type instance, stripped of type information.
+struct UntypedBuiltinValue {


And now I realize this doesn't represent builtin ptr types I think? IIRC it was my intention that it would; we'd just encode it into the UInt128…

UntypedBuiltinValue should represent builtin ptr. But encoding to UInt128 might not be possible. As we could have instruction like this:

%i0.0: &[{}] (sink Int) let -> Int = alloc_stack [{}] (sink Int) let -> Int %i0.1: ptr = address_to_pointer @FunctionDecl(524290) %i0.2: &ptr = subfield_view %i0.0, 0 %i0.3: &ptr = access [set] %i0.2 store %i0.1, %i0.3 end_access %i0.3

Here thing to note is:

Address should also be able to represent function id.

We need to support "function pointers" too.

IDK if Function.ID can be represented in 128 bits.

I don't think Address should represent function IDs. However, the need to represent capture-less lambdas is real and leaves me more and more convinced that we shouldn't try and stuff different types in here.

dabrahams · 2025-11-07T18:52:08Z

Sources/Interpreter/Interpreter.swift

+    if !a.memoryLayout.type.isBuiltin {
+      return nil
+    }
+    let allocIdx = a.memoryAddress.allocation


Where did this name come from? We don't use abbrevs. Also it's all type information at an even lower level of abstraction than Allocation.ID which is already too low. What role is the value playing in this context? That's what goes in the name. If you can't do better than to reflect type information, use a single-character name like a.

Ah yeah, this is really a bad habit, mostly coming from work related codebases which normalize the same 😞. Trying to improve on it now!!!

dabrahams · 2025-11-07T19:05:30Z

Sources/Interpreter/Interpreter.swift

+  /// Returns address of subfield denoted by `path` stored in field at `a`.
+  mutating func subField(denotedBy path: RecordPath, of a: Address)


denotedBy is a lot of big words with little semantic value. path is just type information.

Suggested change

/// Returns address of subfield denoted by `path` stored in field at `a`.

mutating func subField(denotedBy path: RecordPath, of a: Address)

/// Returns the address of `subField` in the object at `origin`.

mutating func address(of subField: RecordPath, in origin: Address)

My suggestion is imperfect and should eventually be improved. The main reason is that I don't think we know how to talk about or properly name the thing called Address here. There may not be an object there yet. I wonder if it should be called ValueStorage, i.e. the storage for a value.

Regarding ValueStorage, knowing that Function.ID is also an Address (coming from IR instruction), I am not sure if ValueStorage would be good.

Also, shouldn't we use at instead of in in function name? Then it would match doc summary too.

dabrahams · 2025-11-13T18:39:38Z

Sources/Interpreter/Memory.swift


+  /// Copies `n` bytes from `source` to `destination`.
+  public mutating func copy(
+    bytes n: Int, from source: Memory.Address, to destination: Memory.Address


Suggested change

bytes n: Int, from source: Memory.Address, to destination: Memory.Address

byteCount n: Int, from source: Memory.Address, to destination: Memory.Address

n is not the bytes. However, you could:

Suggested change

bytes n: Int, from source: Memory.Address, to destination: Memory.Address

_ n: Int, bytesFrom source: Memory.Address, to destination: Memory.Address

dabrahams · 2025-11-13T18:53:26Z

Sources/Interpreter/Interpreter.swift

      _ = x
    case let x as Load:
-      _ = x
+      let address = address(denotedBy: x.source)!;


This denotedBy label isn't working for me.

dabrahams · 2025-11-13T18:57:20Z

Sources/Interpreter/Interpreter.swift

+  /// Returns address of object denoted by operand, if any.
+  func address(denotedBy operand: Operand) -> Address? {


Is it the address of the object denoted by the operand, or the address denoted by the operand? The name is not consistent with the contract.

Isn't this just the operand, which must denote an address?

dabrahams · 2025-11-16T23:32:12Z

Sources/Interpreter/Interpreter.swift

 }

-/// The value produced by executing an instruction.
-enum InstructionResult {


Having a named type was a good idea. Maybe just an alias?

dabrahams · 2025-11-16T23:35:51Z

Sources/Interpreter/Interpreter.swift

+  /// Returns builtin value referenced by `operand`, if any.
+  func toBuiltinValue(_ operand: Operand) -> BuiltinValue? {


Don't repeat type information in the names of non-types

Suggested change

/// Returns builtin value referenced by `operand`, if any.

func toBuiltinValue(_ operand: Operand) -> BuiltinValue? {

/// Returns the value of `x` if it has a builtin type, or `nil` if it does not.

func builtinValue(_ x: Operand) -> BuiltinValue? {

dabrahams · 2025-11-16T23:37:34Z

Sources/Interpreter/Interpreter.swift

+    )
+  }
+
+  /// Store builtin value `v` at address `a`.


Suggested change

/// Store builtin value `v` at address `a`.

/// Stores `v` in memory at `a`.

dabrahams · 2025-11-16T23:38:40Z

Sources/Interpreter/Interpreter.swift

+  /// Moves the program counter to start of `b`.
+  mutating func jumpTo(_ b: Block.ID) {


Suggested change

/// Moves the program counter to start of `b`.

mutating func jumpTo(_ b: Block.ID) {

/// Sets the program counter to the start of `b`.

mutating func jump(to b: Block.ID) {

Read the swift API guidelines regarding prepositions.

dabrahams · 2025-11-16T23:43:05Z

Sources/Interpreter/Interpreter.swift

+  /// Returns block parameters from operands in `arg`.
+  func blockParams(from arg: Call) -> [Address] {


I don't understand the comment. It probably should not require me to know what block parameters are, since that is the name of the function, except for the abbrev, which you should nix.

I think it returns the addresses of arguments followed by that of the return value space for arg.

arg is a terrible name for a function parameter; of course you're going to pass an argument. Another abbrev. Is it not notionally a function call?

dabrahams

I get the sense that perhaps this PR is trying to do too much all at once. Partly I think so because I find myself wanting to make too many corrections.
Would it make sense to break it into smaller units, so we can make a really clean PR and learn from that?

RishabhRD · 2025-11-29T16:58:04Z

Closing this PR as it is broken into #1775, #1780, #1781, #1782, #1783, #1784 as suggested.

codecov · 2025-11-29T16:58:17Z

Codecov Report

❌ Patch coverage is 86.55914% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.14%. Comparing base (46d7e3f) to head (7e76b30).

Files with missing lines	Patch %	Lines
Sources/Interpreter/BuiltinValue.swift	41.17%	20 Missing ⚠️
Sources/Interpreter/Interpreter.swift	96.06%	5 Missing ⚠️

Additional details and impacted files

@@               Coverage Diff               @@
##           interpreter    #1753      +/-   ##
===============================================
+ Coverage        87.85%   88.14%   +0.29%     
===============================================
  Files              392      393       +1     
  Lines            24618    24734     +116     
===============================================
+ Hits             21627    21803     +176     
+ Misses            2991     2931      -60

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

RishabhRD added 19 commits October 28, 2025 13:23

executing instruction results into InstrunctionResult

bd7e162

fix alloc and dealloc instructions

79b3c0c

access instruction returns an address

fc08a71

make access ad subfield work

8930fc0

register should only store builtin object

03fae32

instruction also provides type

5540797

fix byte offsetting for subfield view

cd3312f

feat: implement load instruction

97df887

no need to store address additionally

844594f

feat: memory_copy implemented

644c565

feat: finally calling function works

e1b3b1d

feat: make jump instruction work

36eb17f

fix: unreal abi type layout calculation for bool

7d5c065

feat: store instruction works now

329a953

remove constant_string implementation as it was wrong

b70ed18

Unify stack and heap addresses

038a88f

remove unused stack tests

1e258ed

remove #expect for checking nothrow

eb3f50d

Merge remote-tracking branch 'base/interpreter' into interpreter_func…

46f173f

…tion_call

RishabhRD mentioned this pull request Oct 30, 2025

Interpreter: Make the function call and other basic instructions work #1752

Closed

RishabhRD changed the title ~~Interpreter: Make function calls working~~ Interpreter: Make function calls and basic instructions working Oct 30, 2025

RishabhRD requested a review from dabrahams October 30, 2025 10:16

dabrahams reviewed Oct 30, 2025

View reviewed changes

Sources/Interpreter/TargetABI.swift Show resolved Hide resolved

dabrahams reviewed Oct 31, 2025

View reviewed changes

Sources/Interpreter/Interpreter.swift Outdated Show resolved Hide resolved

dabrahams reviewed Oct 31, 2025

View reviewed changes

Sources/Interpreter/Interpreter.swift Outdated Show resolved Hide resolved

dabrahams reviewed Oct 31, 2025

View reviewed changes

moved stackframe to its original location

78f918b

memory operations is independent of stack and heap

c19ee8c

dabrahams reviewed Nov 6, 2025

View reviewed changes

Sources/Interpreter/Interpreter.swift Outdated Show resolved Hide resolved

dabrahams reviewed Nov 6, 2025

View reviewed changes

safe accessors for type stored at offset

415c1f9

dabrahams reviewed Nov 7, 2025

View reviewed changes

RishabhRD added 5 commits November 8, 2025 18:17

fix: preconditions on memory tests

807209d

added Memory.copy(bytes:,from:,to:) to support copying empty structs

1a89883

apply jumpTo name suggestion from Dave

bf24841

Improve naming and documentation

6d1a4b6

improve using UntypedBuiltinValue

9e4854c

dabrahams reviewed Nov 13, 2025

View reviewed changes

RishabhRD added 6 commits November 16, 2025 20:50

Merge branch 'interpreter' into interpreter_function_call

e7f854d

remove InstructionResult and let it evolve with design

fbd247c

stackframe should contain local variables

ceeb73b

remove untyped builtin value

9cfa91a

remove the use of denotedBy

c2b44ab

address of subfield in parent field

7e76b30

dabrahams reviewed Nov 16, 2025

View reviewed changes

RishabhRD closed this Nov 29, 2025

RishabhRD deleted the interpreter_function_call branch November 29, 2025 16:58

	public var uint8: UInt8? { size == 1 ? UInt8(truncatingIfNeeded: asUInt128) : nil }
	public var i8: UInt8? { size == 1 ? UInt8(truncatingIfNeeded: asUInt128) : nil }

		/// Returns address of subfield denoted by `path` stored in field at `a`.
		mutating func subField(denotedBy path: RecordPath, of a: Address)

	bytes n: Int, from source: Memory.Address, to destination: Memory.Address
	byteCount n: Int, from source: Memory.Address, to destination: Memory.Address

	bytes n: Int, from source: Memory.Address, to destination: Memory.Address
	_ n: Int, bytesFrom source: Memory.Address, to destination: Memory.Address

		/// Returns address of object denoted by operand, if any.
		func address(denotedBy operand: Operand) -> Address? {

		/// Returns builtin value referenced by `operand`, if any.
		func toBuiltinValue(_ operand: Operand) -> BuiltinValue? {

	/// Store builtin value `v` at address `a`.
	/// Stores `v` in memory at `a`.

		/// Moves the program counter to start of `b`.
		mutating func jumpTo(_ b: Block.ID) {

		/// Returns block parameters from operands in `arg`.
		func blockParams(from arg: Call) -> [Address] {


		}

		struct ObjectAddress {

Interpreter: Make function calls and basic instructions working #1753

Interpreter: Make function calls and basic instructions working #1753

Uh oh!

Conversation

RishabhRD commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Design Changes

Unified Memory Representation

Design

Instructions implemented:

Instructions UN-implemented

Uh oh!

Uh oh!

Uh oh!

dabrahams Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dabrahams left a comment

Choose a reason for hiding this comment

Uh oh!

RishabhRD commented Oct 31, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dabrahams Nov 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dabrahams left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RishabhRD commented Nov 29, 2025

Uh oh!

codecov bot commented Nov 29, 2025

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

RishabhRD commented Oct 30, 2025 •

edited

Loading

dabrahams Oct 31, 2025 •

edited

Loading

dabrahams Nov 16, 2025 •

edited

Loading

dabrahams left a comment •

edited

Loading