Skip to content

[RV64_DYNAREC] SET_ELEMENT_WIDTH always call vector_vsetvli#3615

Draft
zqb-all wants to merge 1 commit intoptitSeb:mainfrom
zqb-all:rvv-management
Draft

[RV64_DYNAREC] SET_ELEMENT_WIDTH always call vector_vsetvli#3615
zqb-all wants to merge 1 commit intoptitSeb:mainfrom
zqb-all:rvv-management

Conversation

@zqb-all
Copy link
Contributor

@zqb-all zqb-all commented Mar 5, 2026

SET_ELEMENT_WIDTH always call vector_vsetvli

@ptitSeb ptitSeb requested a review from ksco March 5, 2026 08:14
@ptitSeb
Copy link
Owner

ptitSeb commented Mar 5, 2026

I guess your are vibe coding here.

For the 1. you added code just after fpu_pushcache(dyn, ninst, reg, 0); which purpose is to just do what you described.

Copy link
Owner

@ptitSeb ptitSeb left a comment

Choose a reason for hiding this comment

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

Use fpu_pushcache(...) (and don't forget fpu_popcache(...) instead of forgetting all SSE regs.

@zqb-all
Copy link
Contributor Author

zqb-all commented Mar 5, 2026

I guess your are vibe coding here.

Yes, claude helped generate some code.

For the 1. you added code just after fpu_pushcache(dyn, ninst, reg, 0); which purpose is to just do what you described.

Oh, my mistake, I had seen that before, but later when I realized I needed to Forget all registers, I forgot about it. I'll go and make the changes.

@ptitSeb
Copy link
Owner

ptitSeb commented Mar 5, 2026

The not07 is here because, when calling a native function, XMM0..XMM7 are potential arguments and will not be preserve accross call, according to the ABI. So the not07 is here to not save those args as they can be clobered as part of the ABI.
At least that's how ARM64 work, where XMM0..XMM7 are mapped to the sames regs use to pass SIMD type of argument to functions. It should be the same on RV64, but needs to be checked.

TL;DR: you are sure your issue is the clober of the XMM0..7 regs? and not just the vector setup instead?

@zqb-all
Copy link
Contributor Author

zqb-all commented Mar 5, 2026

You're right, sorry for the confusion — I wasn't familiar enough with the x86 ABI and didn't verify carefully before commit. Thanks for the clarification, I now have a much better understanding of the code. I'll update the patch soon after some wemeet test.

@zqb-all zqb-all marked this pull request as draft March 5, 2026 16:35
@zqb-all zqb-all changed the title [RV64_DYNAREC] Fix vector state management for native calls RV64_DYNAREC] SET_ELEMENT_WIDTH always call vector_vsetvli Mar 5, 2026
@zqb-all zqb-all changed the title RV64_DYNAREC] SET_ELEMENT_WIDTH always call vector_vsetvli [RV64_DYNAREC] SET_ELEMENT_WIDTH always call vector_vsetvli Mar 5, 2026
@zqb-all
Copy link
Contributor Author

zqb-all commented Mar 5, 2026

I previously thought I had found the cause, but after further testing, simply modifying the SET_ELEMENT_WIDTH can also avoid the wemeet crash. Now I'm not entirely sure if this is the final solution, so I've set it as WIP for now.

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