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>
bjorn3 commented on Issue #1607:
Will this still work if
alloca
is introduced in the future?
cfallin commented on Issue #1607:
Thanks! Fixed the comment nits.
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 fixedfp
/lr
frame below the stackslots/clobbers since aarch64 allows that flexibility... for now, though, since we don't havealloca
, I think I'll want to save this as a future improvement.
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
.
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.
cfallin commented on Issue #1607:
Pushed a rebase over the recent file-split commit just now.
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
?) -- usingtmp2
now and added a comment to justify why it's safe.
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: Dec 23 2024 at 12:05 UTC