Stream: git-wasmtime

Topic: wasmtime / issue #4485 x64: Port func_addr to ISLE

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2022 at 18:20):

cfallin commented on issue #4485:

Additionally, change the name argument for LoadExtAddr from a Box<ExternalName> to ExternalName, as it greatly simplifies the interface with ISLE, and there didn't appear to be a downside.

Can you check the size of the resulting Inst enum? That's likely the reason we did the boxing; larger insts have measurable and pretty significant compile-performance implications (higher cache footprint, more memory traffic).

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2022 at 19:03):

github-actions[bot] commented on issue #4485:

Subscribe to Label Action

cc @cfallin, @fitzgen

<details>
This issue or pull request has been labeled: "cranelift", "cranelift:area:machinst", "cranelift:area:x64", "isle"

Thus the following users have been cc'd because of the following labels:

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2022 at 19:56):

elliottt commented on issue #4485:

Additionally, change the name argument for LoadExtAddr from a Box to ExternalName, as it greatly simplifies the interface with ISLE, and there didn't appear to be a downside.

Can you check the size of the resulting Inst enum? That's likely the reason we did the boxing; larger insts have measurable and pretty significant compile-performance implications (higher cache footprint, more memory traffic).

That change didn't affect the size of the x64 Inst, however as it was already 72 bytes I went ahead and switched back to BoxExternalName to not add extra work for whoever decides to start optimizing for size.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2022 at 19:57):

elliottt edited a comment on issue #4485:

Can you check the size of the resulting Inst enum? That's likely the reason we did the boxing; larger insts have measurable and pretty significant compile-performance implications (higher cache footprint, more memory traffic).

That change didn't affect the size of the x64 Inst, however as it was already 72 bytes I went ahead and switched back to BoxExternalName to not add extra work for whoever decides to start optimizing for size.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2022 at 19:57):

elliottt edited a comment on issue #4485:

Can you check the size of the resulting Inst enum? That's likely the reason we did the boxing; larger insts have measurable and pretty significant compile-performance implications (higher cache footprint, more memory traffic).

That change didn't affect the size of the x64 Inst, however as it was already 72 bytes I went ahead and switched back to BoxExternalName to not add extra work for whoever decides to start optimizing for size. I also made another PR that adds a test for x64 instruction size so that we can avoid slipping further #4489.

Last updated: Nov 22 2024 at 16:03 UTC