Skip to content

V_CMP_U64 fix#4130

Closed
georgemoralis wants to merge 13 commits intoshadps4-emu:mainfrom
georgemoralis:rec_fixes1
Closed

V_CMP_U64 fix#4130
georgemoralis wants to merge 13 commits intoshadps4-emu:mainfrom
georgemoralis:rec_fixes1

Conversation

@georgemoralis
Copy link
Copy Markdown
Collaborator

handle src1 as VectorGPR in V_CMP_U64
This should fix some vector_alu.cpp asserts

@squidbus
Copy link
Copy Markdown
Collaborator

This doesn't look right to me, we're no longer treating SGPRs as per-thread bits?

@squidbus
Copy link
Copy Markdown
Collaborator

(cc: @raphaelthegreat )

@georgemoralis
Copy link
Copy Markdown
Collaborator Author

ok added an implemenation for avoid using u64 emitter , should be more optimal and correct @squidbus if you agree with this :D

@squidbus
Copy link
Copy Markdown
Collaborator

squidbus commented Mar 17, 2026

Currently you're still using GetScalarReg, before we were using GetThreadBitScalarReg. The difference being that the latter will use the single bit of the register belonging to each shader thread in the wave to compare, since typically that is how these 64-bit compares are used with scalar registers; they are comparing the 64 bits coming from all the wave threads.

It's possible for this to not be the case and it to be a normal 64-bit compare I suppose, but you can't really ignore the thread bit case.

@raphaelthegreat
Copy link
Copy Markdown
Contributor

You seem to have reimplemented this as a 64-bit op which is incorrect, in vast majority of cases this should use the thread bit values which is what I had fixed in my PR. Usage of VGPRs are inputs is indicative of needing 64-bit usage, but currently that would also break because v_mov_b32 can't handle a thread bit source.

@Kravickas
Copy link
Copy Markdown
Contributor

how about S_MOV_B64 as an example for this V_CMP_U64 ?

@georgemoralis
Copy link
Copy Markdown
Collaborator Author

will be fixed with another pr shortly

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.

5 participants