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

[X86][Disassembler] vpsubq (%esp), %xmm1, %xmm2 can not be decoded by LLVM's disassembler #54540

Closed
KanRobert opened this issue Mar 25, 2022 · 9 comments
Assignees

Comments

@KanRobert
Copy link
Contributor

bash$ cat 1.s
vpsubq (%esp), %xmm1, %xmm2
bash$ llvm-mc --filetype=obj 1.s -o 1.o
bash$ llvm-objdump -d 1.o

1.o:    file format elf64-x86-64Disassembly of section .text:0000000000000000 <.text>:
       0: 67 c5 f1 fb 14 24             <unknown>
@KanRobert KanRobert self-assigned this Mar 25, 2022
@KanRobert
Copy link
Contributor Author

Ongoing patch: https://reviews.llvm.org/D122448

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 25, 2022

@llvm/issue-subscribers-backend-x86

@hvdijk
Copy link
Contributor

hvdijk commented Mar 25, 2022

Thanks for the clear example. The patch that you are reverting fixes an ICE-on-valid-C++ in clang that shows up when building Qt though, so we are in a lousy situation where something important is broken if we do revert, something else also important is broken if we don't. I cannot comment on the relative priorities, I do not know which is worse. I expect to have time over the weekend to look into whether this is easy to fix so that both work, is it okay to wait for that?

@KanRobert
Copy link
Contributor Author

@hvdijk Sure. Let me know if you need my help. I'm familiar w/ X86 encoding/decoding and able to provide a quick solution to support both of them.

@KanRobert
Copy link
Contributor Author

https://reviews.llvm.org/D122537
https://reviews.llvm.org/D122448

These two patches can fix both of them.

@hvdijk
Copy link
Contributor

hvdijk commented Mar 27, 2022

Regardless of whether that would be enough, we cannot commit that as there is a conflict with another change (also of yours) that has gone in. I am working on resolving the conflict and I will submit a patch soon. I understand that you can resolve the conflict, but you know I'd already started work on this, please let me finish that. (Edit: while the expression of frustration was intended, reading back it looks like going beyond that to accusations. That wasn't what I was trying for. Sorry about that, I've edited.)

@hvdijk
Copy link
Contributor

hvdijk commented Mar 27, 2022

Created https://reviews.llvm.org/D122540, tested with your https://reviews.llvm.org/D122449 applied on top.

@hvdijk
Copy link
Contributor

hvdijk commented Apr 12, 2022

Thanks for the patience, the fix is now in on main. What would you prefer for the branches? The fix should not cherry-pick cleanly, so we can either decide to leave those as they are or make the required changes. I am fine with either, and can make those changes if you like.

@KanRobert
Copy link
Contributor Author

I work on the main branch most of the time, so I don't have any back-porting request. I think no more work is need until we receive a request. Thank you for the effort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants