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

Disassembly issues with v_writelane_b32 #61

Closed
ishitatsuyuki opened this issue Jan 21, 2022 · 7 comments
Closed

Disassembly issues with v_writelane_b32 #61

ishitatsuyuki opened this issue Jan 21, 2022 · 7 comments

Comments

@ishitatsuyuki
Copy link

RADV/ACO can generate v_writelane_b32 with additional src operands, and the LLVM disassembler has issues recognizing it:

	.long 0xd761000b // unrecognized instruction                                                        // 00000000031C: D761000B                                                                           
	.long 0x42d4001 // unrecognized instruction                                                         // 000000000320: 042D4001 

Presumably, this is also preventing the user from viewing the otherwise valid instruction tracing data.

A sample profile is attached for testing.

capture.zip

FWIW, in RADV there's a workaround that masks out the unsupported part:

https://github.com/mesa3d/mesa/blob/b5b105df96d705e5d0bc381c681be4d8120d815f/src/amd/compiler/aco_print_asm.cpp#L274-L278

@chesik-amd
Copy link
Contributor

Thanks for reporting this and providing a test case. We can reproduce this and will work with the disassembler team to investigate.

@chesik-amd
Copy link
Contributor

Our disassembler team took a quick look at this, and the instructions are not encoded correctly. It seems this same issue prompted the aco_print_asm workaround you linked to.

Here is what they said:

**It looks like RADV is generating machine code instructions for writelane with src2 bits set to something random - which it doesn't care about.
When the aco_print_asm code is called, it specifically masks out the src2 bits before it uses the llvm disassembler (the same one we use in amdgpu-dis) to disassemble the instruction, so they already seem to be aware that it's a problem.

So, yes, as far as amdgpu-dis is concerned, this encoding is not legal.

Agree that passing the info back to the originator seems like the best thing to do.**

Can you work with the RADV team on fixing this at the source?

@Venemo
Copy link

Venemo commented Feb 2, 2022

@chesik-amd I work at the RADV team.

We can skip encoding this field for this instruction, but it feels like this is just to satisfy the disassembler, rather than an actual issue with the correctness of the instruction.

Our understanding is that this encoding is valid because the GPUs can execute it without issue and the ISA documentation also doesn't say this would be invalid.

@daniel-schuermann
Copy link

It looks like RADV is generating machine code instructions for writelane with src2 bits set to something random

Not random, it's the def register, that we internally have as src2 to maintain SSA ;)
But we can add a workaround, sure thing.

@chesik-amd
Copy link
Contributor

Closing this issue, as it sounds like this has been (or will be?) addressed by the RADV team.

@Venemo
Copy link

Venemo commented May 31, 2022

@chesik-amd Yes, I can confirm we worked around this issue in RADV some time ago.

@chesik-amd
Copy link
Contributor

@Venemo Thank you for confirming

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

No branches or pull requests

4 participants