Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement the fneg instruction. #1462

Merged
merged 1 commit into from
Nov 12, 2024
Merged

Implement the fneg instruction. #1462

merged 1 commit into from
Nov 12, 2024

Conversation

vext01
Copy link
Contributor

@vext01 vext01 commented Nov 11, 2024

Required for LuaJIT tests.

Please carefully check the register allocator changes. I had to implement RegConstraint::Temporary in assign_fp_regs().

Requires: ykjit/ykllvm#214

; %3: double = fneg %1
{{_}} {{_}}: mov r.64.x, 0x8000000000000000
{{_}} {{_}}: movq fp.128.y, r.64.x
{{_}} {{_}}: xorpd fp.128.a, fp.128.y
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was curious that %3 was allocated a fresh register (fp.128.a), instead of reusing the register previously assigned to the now-dead %2 (fp.128.z). Could indicate an error in my register allocator changes?

Copy link
Contributor

Choose a reason for hiding this comment

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

We currently prefer to allocate "higher" registers, which is pointless in FP-land, but in GP-land means we're less likely to get twonked by x64's fondness for RAX and the sysv abi's preference for "lower" registers in CALLs. Does that guarantee what you're seeing is intended? Nope. But it might give you a clue to what look for.

@ltratt
Copy link
Contributor

ltratt commented Nov 11, 2024

LGTM.

@ltratt ltratt requested review from jacob-hughes and removed request for jacob-hughes November 11, 2024 16:24
@vext01
Copy link
Contributor Author

vext01 commented Nov 11, 2024

LLVM synced. OK to squash?

@ltratt
Copy link
Contributor

ltratt commented Nov 11, 2024

Please squash.

Required for LuaJIT tests.
@vext01
Copy link
Contributor Author

vext01 commented Nov 12, 2024

squashed.

@ltratt ltratt added this pull request to the merge queue Nov 12, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 12, 2024
@vext01
Copy link
Contributor Author

vext01 commented Nov 12, 2024

Once #1463 is in, this should merge. It's the ambiguous methods warning again.

@ltratt ltratt added this pull request to the merge queue Nov 12, 2024
@vext01 vext01 removed this pull request from the merge queue due to a manual request Nov 12, 2024
@vext01
Copy link
Contributor Author

vext01 commented Nov 12, 2024

Not yet :)

@ltratt
Copy link
Contributor

ltratt commented Nov 12, 2024

I deliberately merged both at once, because CI can run the merge together. There is no harm in doing so.

@ltratt ltratt added this pull request to the merge queue Nov 12, 2024
Merged via the queue into ykjit:master with commit 3179eb5 Nov 12, 2024
2 checks passed
@vext01 vext01 deleted the fneg branch November 12, 2024 11:08
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.

2 participants