Skip to content

interp: limit each value to its own bytes in toLLVMValue - #5826

Open
0pcom wants to merge 2 commits into
tinygo-org:devfrom
0magnet:interp-tollvm-element-bounds
Open

0pcom wants to merge 2 commits into
tinygo-org:devfrom
0magnet:interp-tollvm-element-bounds

Conversation

@0pcom

@0pcom 0pcom commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

toLLVMValue gave each struct field and array element the rest of the buffer from its offset, and the zero check at the top scanned all of it. Serializing a large global that an init function has written to was quadratic in its size.

github.com/mattn/go-runewidth has a [2][0x110000]byte width table that its init writes the start of. Programs that import it, for example through charmbracelet/x/ansi, spent about 26 minutes in interp. With this change it takes under a second and the output is unchanged.

The new test uses a global of the same shape. It passes in 0.2s and did not finish in 3 minutes without the change. This is similar to #5368, which fixed a rescan of the same kind in memoryView.store.

Struct fields and array elements were given the rest of the buffer, so the
zero check scanned far past them. A large global that an init function
writes to then took quadratic time to serialize. The width table in
go-runewidth took about half an hour, and now takes under a second.

@dgryski dgryski left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable but I wouldn't mind a second look from someone who understands interp more.

@jakebailey

Copy link
Copy Markdown
Member

The regression test covers the array path, but not the changed struct field slicing. This could go in interp/testdata/store.ll, to cover padded fields and a nested struct:

@struct.value = global { i8, i64, { i32, i8 } } zeroinitializer

define internal void @struct.init(ptr %context) unnamed_addr {
entry:
  %b = getelementptr { i8, i64, { i32, i8 } }, ptr @struct.value, i32 0, i32 1
  store i64 42, ptr %b
  %e = getelementptr { i8, i64, { i32, i8 } }, ptr @struct.value, i32 0, i32 2, i32 1
  store i8 7, ptr %e
  ret void
}

With call void @struct.init(ptr undef) added to runtime.initAll, the expected initializer in store.out.ll is:

@struct.value = global { i8, i64, { i32, i8 } } { i8 0, i64 42, { i32, i8 } { i32 0, i8 7 } }

The existing test only reached the array element path. Add a padded
struct with a nested struct so the struct field slicing in
toLLVMValue is exercised too.
@0pcom

0pcom commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the IR. I added the struct test to store.ll and store.out.ll as you gave it, with local_unnamed_addr on the output global to match the other entries in that file. It now covers the padded field and the nested struct that the array test could not reach. I did not build or run it locally because this machine has no LLVM build for the tree, so CI is the check.

@0pcom

0pcom commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

I have now built and run it. On LLVM 23 (on top of #5782) go test ./interp passes, including the new struct case in store.ll and TestInterpLargeArray (0.13s).

The struct case also passes on the code before this change, which is expected. The old path was slow but gave the same output, so the test guards the output of the new slicing for padded and nested fields.

@0pcom 0pcom closed this Oct 8, 2026
@0pcom 0pcom reopened this Oct 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants