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

[RV64_DYNAREC] Added 66 0F D4 PADDQ opcode for vector and fixes SEW cache transform #1812

Merged
merged 2 commits into from
Sep 10, 2024

Conversation

ksco
Copy link
Collaborator

@ksco ksco commented Sep 10, 2024

No description provided.

@ptitSeb
Copy link
Owner

ptitSeb commented Sep 10, 2024

Why is the ordering of the sewTranform maters?

@ksco
Copy link
Collaborator Author

ksco commented Sep 10, 2024

Why is the ordering of the sewTranform maters?

Because it changes the runtime effective sew of the vector unit, but do not change our compile time tracking of sew for current instruction.

So for example, if the current compile time sew is 64, and it changed to 32 in the sew transformation, if there is a VLE instruction in the fpu transformation it will be VLE64 as the compile time sew is 64, so the instruction will SIGILL because it runtime sew has already changed to 32. (It may seems strange, but VLE64 requires current sew to be 64.)

@ptitSeb
Copy link
Owner

ptitSeb commented Sep 10, 2024

Why is the ordering of the sewTranform maters?

Because it changes the runtime effective sew of the vector unit, but do not change our compile time tracking of sew for current instruction.

So for example, if the current compile time sew is 64, and it changed to 32 in the sew transformation, if there is a VLE instruction in the fpu transformation it will be VLE64 as the compile time sew is 64, so the instruction will SIGILL because it runtime sew has already changed to 32. (It may seems strange, but VLE64 requires current sew to be 64.)

Ok, but what happens if the sew needs change in fpuTransform but sewTransform is not needed?

@ksco
Copy link
Collaborator Author

ksco commented Sep 10, 2024

Why is the ordering of the sewTranform maters?

Because it changes the runtime effective sew of the vector unit, but do not change our compile time tracking of sew for current instruction.
So for example, if the current compile time sew is 64, and it changed to 32 in the sew transformation, if there is a VLE instruction in the fpu transformation it will be VLE64 as the compile time sew is 64, so the instruction will SIGILL because it runtime sew has already changed to 32. (It may seems strange, but VLE64 requires current sew to be 64.)

Ok, but what happens if the sew needs change in fpuTransform but sewTransform is not needed?

The fpuTransform is and will always be sew agnostic, meaning any valid sew is okay for it, so there will never be any sew changes in fpuTransform.

@ptitSeb
Copy link
Owner

ptitSeb commented Sep 10, 2024

This seems a bit odd to me, but that's ok. a SIGILL is easy enough to catch and diagnose if something goes wrong later.

@ptitSeb ptitSeb merged commit b0ec124 into ptitSeb:main Sep 10, 2024
23 checks passed
@ksco ksco deleted the more branch September 10, 2024 08:52
@ksco
Copy link
Collaborator Author

ksco commented Sep 10, 2024

This seems a bit odd to me, but that's ok.

Things in the fpuCacheTransform are basically moves, loads and stores, so it’s natural that it’s not bound to a specific SEW, which is also good for performance.

@ptitSeb
Copy link
Owner

ptitSeb commented Sep 10, 2024

This seems a bit odd to me, but that's ok.

Things in the fpuCacheTransform are basically moves, loads and stores, so it’s natural that it’s not bound to a specific SEW, which is also good for performance.

But yet, it still needs sewTranform to be done after and not before?

@ksco
Copy link
Collaborator Author

ksco commented Sep 10, 2024

I guess we can “fix” this by actually change the compile time sew in sewTransform in and only in pass3.

@ptitSeb
Copy link
Owner

ptitSeb commented Sep 10, 2024

that's not really my point. I'm just woried that this "need" is just the manifastation of an issue where some needed sew tranform is needed and missed...

@ksco
Copy link
Collaborator Author

ksco commented Sep 10, 2024

ah ok, got it.

JunChi1022 pushed a commit to JunChi1022/box64 that referenced this pull request Sep 12, 2024
…ache transform (ptitSeb#1812)

* [RV64_DYNAREC] Added 66 0F D4 PADDQ opcode for vector

* [RV64_DYNAREC] Transform SEW cache after fpu cache
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