-
Notifications
You must be signed in to change notification settings - Fork 18k
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/internal/asm: add encoding tests for all architectures #18072
Comments
I don't think this is a release blocker. |
I disagree mildly. Some time recently the tests were weakened, and they should be restored to the strength they had in prior releases. |
I spoke to Rob earlier. He was remembering the fact that there used to be a golden x.out next to each x.s in the testdata directory; the x.out contained the output from 'asm -S x.s'. These proved too annoying to keep in sync, so in https://golang.org/cl/18823 I changed the test to require instead that, by default, each line found in the assembly is reprinted with exactly the same syntax. That is, the result of the parse is tested and must reprint as the original input exactly. When there are multiple equivalent forms, so that the result of parse+reprint may vary, a comment in the testdata file specifies the desired printed form. For example:
The middle operand is omitted in the input but defaults to the final one, so that's what is printed and what is expected by the test (due to the comment). In a followup CL, https://golang.org/cl/18844 and https://golang.org/cl/18850, I added support for an optional comment giving the hex instruction encoding too, formerly not tested at all:
The two can be combined if needed, as in It is true the tests are too weak: there should be hex instruction encoding tests for all architectures, not just amd64 and s390x. But this work is unplanned. |
On a related note, is there a test that verifies the correct pc-line info for assembly output? If not, that would also be nice to have. |
@griesemer We have some indirect tests. For example test/nilptr3.go tests (among other things) that line numbers are right, at least up to the nil pointer removal compiler phase. |
After CL 45996 is merged, I thought I have made a relative full encoding test for ARM32. Some corner cases are not tested, since they use the constant pool, which is hard to calculate their offset by hand. |
At tip as of Nov 28, 2016: The end-to-end tests in cmd/asm/internal/asm/testdata are much weaker than they used to be. I have vague memories of a change someone (Russ?) made there, but now for most architectures all that is tested is the parse, not the result of the parse. These tests should be verifying that the assembler produces correct output, not just that it accepts valid input.
The text was updated successfully, but these errors were encountered: