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
cmd/internal/obj: wrong code generated by the arm assembler #19141
Comments
While MULAWT is right, MULAWT -> e1 24 31 c2 |
I found this issue when tried to add test cases for my CL https://go-review.googlesource.com/#/c/35565/ |
I have fixed this issue in patch set 15 of https://go-review.googlesource.com/#/c/35565/ The compiler also needs modification, since it followed the assembler's error in MULA. |
CL https://golang.org/cl/37021 mentions this issue. |
CL https://golang.org/cl/35565 mentions this issue. |
It would be good and consistent that the last register is the destination. But I don't think this is documented and required. MULA has been there for a long time and there are probably user assembly code that rely on this encoding. Maybe just leave it as is? |
But MULAWB MULAWT use the last register as destination. That would confuse the user. Maybe we would add other MULA serial instructions, like Multiply and Subtract MLS 32 = 32 – 32 × 32 How about these ones? Use the first as destination like MULA or use the last like MULAWB / MULAWT? |
Now I also agree with Cherry that we keep current circumstance unchanged. Since a change in the assembler will cause For others MULS, MMULA, MMULS, MULABB, MULAWT, MULAWB, they can follow MULA, use the third register as destination and the fourth as the addend. |
No, I didn't mean that we should change everything to the "wrong" MULA encoding. MULAWT, MULAWB are in the "correct" encoding, which uses last register as destination. They should keep unchanged. The new instructions should follow the correct encoding. I think we'd want MULA fixed. But it needs to be done more carefully, since there might be user code that uses it. |
If we want everything be in the right order, the disassembler / objdump also need change. Otherwise tests in arm.s will fail, since it check both the output's 32-bit binary code and the disassembled text.
|
Now CL https://go-review.googlesource.com/#/c/35565/ has nothing to do with this issue. Since I And I will create a new CL to fix this issue. |
CL https://golang.org/cl/42028 mentions this issue. |
Fixed in CL https://golang.org/cl/42028 |
The MULA is wrongly assembled by the ARM assembler, even the newest go.
For example, MULA r1, r2, r3, r4 is assembled to e0 23 41 92, which should be e0 24 31 92.
The addend is r3 and the result is r4, the assembler confuses them.
The text was updated successfully, but these errors were encountered: