Skip to content

Conversation

@vext01
Copy link
Contributor

@vext01 vext01 commented Mar 13, 2025

Requires ykjit/yk#1651

@vext01
Copy link
Contributor Author

vext01 commented Mar 19, 2025

@ltratt do you think this one should go in too? Is it now trusted now that we think the problem was the ykllvm bug?

@ltratt
Copy link
Contributor

ltratt commented Mar 19, 2025

"Yes but":

  1. This will almost certainly have a merge conflict with Use a version number for idempotency. #107.
  2. I'd ideally like to benchmark before/after this, just to see if it makes any difference. [I'm not expecting much of a difference, but am slightly curious.]

@vext01
Copy link
Contributor Author

vext01 commented Mar 19, 2025

OK. I'll hold off.

I don't expect a large performance change, but let's see. If anything the removal of bitcast instructions should be in our favour.

@ltratt
Copy link
Contributor

ltratt commented Mar 19, 2025

Please merge in master and address the merge conflict. Because this one's quite subtle, I think a force push is not the way to go here.

@vext01
Copy link
Contributor Author

vext01 commented Mar 19, 2025

I think this is correct. The lua tests are passing.

@ltratt
Copy link
Contributor

ltratt commented Mar 19, 2025

Looks good! Please squash.

@vext01
Copy link
Contributor Author

vext01 commented Mar 19, 2025

squashed

@ltratt ltratt added this pull request to the merge queue Mar 19, 2025
Merged via the queue into ykjit:main with commit c0415f4 Mar 19, 2025
2 checks passed
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