Add dedicated ptrtoint and intoptr bytecodes. #1639
Conversation
| ... | ||
| ; %0: ptr = param ... | ||
| ; %1: i64 = ptr_to_int %0 | ||
| mov rcx, rax |
There was a problem hiding this comment.
It's not obvious at first, but this is the register allocator moving the value so it can reuse it (the underlying operation is really a no-op), so it's not guaranteed to put them in rcx or whatnoe. So we really want to abstract the test a bit to something like:
; %1
mov r.64.a, rax
; %2
mov r.64.b, r.64.a
; %3
mov r.64.c, r.64.bNow all that said: when we improve the register allocator further, it shouldn't need to allocate any registers!
There was a problem hiding this comment.
we really want to abstract the test a bit
Oops. My bad. I did it in the other tests, but not in this one.
when we improve the register allocator further, it shouldn't need to allocate any registers!
Agreed.
By the way, also notice that there is no comment for %4 in the asm output? I assume this is a quirk of how we format the asm code. Perhaps if there was one more asm instruction in the listing the comment for %4 would be attached to that? Not sure if this is worth investigating or not.
There was a problem hiding this comment.
The %4 thing is a known quirk: comments are printed before an instruction, and that particular instruction doesn't generate any code. We could do something different here, but it's not a matter for this PR IMHO.
|
Force pushed a fix and synced ykllvm. |
|
I am a big dummy, so the non-alphabetical ordering of c/a/d/b is confusing for my pea brain: I had to read it twice to work out what was going on. Can we use an alphabetical ordering please? |
|
Force pushed a fix that names the registers alphabetically in order of assignment. |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Before we lowered a subset of LLVM ptrtoint and inttoptr instructions to zext instructions in our AOT IR. This was confusing, so this change makes proper opcodes for those operations (and tests them).
|
Force pushed clippy fixes. |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
Richards timed out. Could be nothing to do with this PR? Shall we retry? |
Requires ykjit/ykllvm#232 (and an LLVM sync)
Before we lowered a subset of LLVM ptrtoint and inttoptr instructions to zext instructions in our AOT IR.
This was confusing, so this change makes proper opcodes for those operations (and tests them).