Stream: git-wasmtime

Topic: wasmtime / Issue #1607 Rework aarch64 stack frame impleme...

Wasmtime GitHub notifications bot (Apr 27 2020 at 21:51):

github-actions[bot] commented on Issue #1607:

Subscribe to Label Action

cc @bnjbvr

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

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

• bnjbvr: cranelift

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

Learn more.
</details>

Wasmtime GitHub notifications bot (Apr 28 2020 at 08:54):

bjorn3 commented on Issue #1607:

Will this still work if alloca is introduced in the future?

Wasmtime GitHub notifications bot (Apr 29 2020 at 16:05):

cfallin commented on Issue #1607:

Thanks! Fixed the comment nits.

Wasmtime GitHub notifications bot (Apr 29 2020 at 16:35):

cfallin commented on Issue #1607:

@bjorn3:

Will this still work if alloca is introduced in the future?

That's a good point; we'll need to do something else in this case. I'll have to look at what other compilers do on aarch64 -- perhaps they just accept the cost of negative offsets from fp, or perhaps they put the fixed fp/lr frame below the stackslots/clobbers since aarch64 allows that flexibility... for now, though, since we don't have alloca, I think I'll want to save this as a future improvement.

Wasmtime GitHub notifications bot (Apr 29 2020 at 16:46):

bjorn3 commented on Issue #1607:

for now, though, since we don't have alloca, I think I'll want to save this as a future improvement.

:+1: I just wanted to prevent any decisions that would make it much harder to support alloca.

Wasmtime GitHub notifications bot (Apr 29 2020 at 16:54):

cfallin commented on Issue #1607:

@bjorn3: yup, thanks for bringing it up, as it hadn't occurred to me! One solution (to record my thought for the future) might be to simply allocate a vreg as our "stackslot area base" and copy SP before we do any dynamic adjustments, and perhaps revert to this only if, on a scan, we see that an alloca is present.

Wasmtime GitHub notifications bot (Apr 30 2020 at 23:22):

cfallin commented on Issue #1607:

Pushed a rebase over the recent file-split commit just now.

Wasmtime GitHub notifications bot (May 05 2020 at 18:23):

cfallin commented on Issue #1607:

Thanks! Updated, PTAL.

I think lower_addr may reuse the temporary spill registers as well, can you please take a look?

Fixed (I assume you meant load_addr?) -- using tmp2 now and added a comment to justify why it's safe.

Wasmtime GitHub notifications bot (May 05 2020 at 18:26):

cfallin commented on Issue #1607:

And, I should note I built a SpiderMonkey JS shell with this commit vendored in, and all jit-tests pass; so I think the Baldrdash-related stuff is still correct.

Last updated: Nov 22 2024 at 17:03 UTC