-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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/asm: s390x tests are suspicious #18295
Comments
Fixing the printing sounds good to me. We could also modify the order that asmz.go expects, avoiding the re-ordering entirely. (This would entail placing the first operand into Some background: This ordering stems from the implementation of the storage-to-storage instructions. For example, When implementing the vector instructions it seemed to make sense to stick to the same convention, placing the length/index (the first operand, if it exists) into There is no particular reason anymore to keep this ordering in the back-end (AFAIK 1.9 seems reasonable, since this only really impacts people writing s390x assembly (except for |
Thanks for the background. I agree that fixing the printing seems best. (I'd rather not invalidate all the existing assembly.) Whether you fix the printing by a special case in the printing function or by changing the internal representation is up to you. |
CL https://golang.org/cl/40299 mentions this issue. |
cmd/asm/internal/asm/testdata/s390x.s says:
This seems wrong. The first column is the assembler input. The second column is the result of parsing column 1 and then printing it back. The second column is supposed to be semantically identical to the first, different only if the assembler printing uses a different spelling than the input. For example if you wrote 'MOV $0x00, R1' then it might reasonably print instead as 'MOV $0, R1'. But clearly the two mean the same thing as asssembler input.
In contrast, it is far from clear to me that 'VSEL V1, V2, V3, V4' and 'VSEL V2, V3, V1, V4' mean the same thing as assembler input. In fact I'd be very surprised if they did.
At this point probably the input meanings cannot be changed, but the printer should be fixed.
Not blocking Go 1.8.
/cc @mundaym @randall77
The text was updated successfully, but these errors were encountered: