search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.85k stars 17.65k forks source link

cmd/asm: how to test? #25724

Open robpike opened 6 years ago

robpike commented 6 years ago

The current tests are an inconsistent mess. Some files are hand-written. Others are auto-generated and enormous, and could be generated on the fly instead of being checked in. Some architectures are well tested. Others are not. Some subsets of architectures (AVX) are exhaustively tested. Others are not.

I would like a proper discussion about how to test the assembler (and the compiler, if you like). Without a clear picture, the tests will become enormous, wildly inconsistent, messy, and difficult to maintain.

quasilyte commented 6 years ago

Thank you for opening an issue.

Please, correct me where I'm wrong.

(quotes from https://go-review.googlesource.com/c/go/+/115858)

From @ianlancetaylor :

I think that Rob is saying that we previously did not have machine generated tests for the assembler. Now, it appears, we do.

We had x86test program written by Russ that generated amd64enc.s test suite. It covered most legacy and some early AVX instructions

Many of those were commented-out with TODO comments, I've implemented missing instructions and performed uncommenting automatically with x86avxgen program (for legacy instruction, this was done by hand).

From @robpike:

Why does AVX get so much attention?

Because legacy x86 instruction already had coverage in amd64enc.s. The second reason is because it was my objective to implement missing early AVX extensions as well as AVX-512.

Some architectures are well tested. Others are not

That may sound unfair, but if I'm working on x86 arch, it makes sense that I care more about that one. At least I expect not to be blamed that other architectures not have auto generated suites right now.

To conclude: x86 test suite has auto generated tests for both new and older instructions. The hand-written + auto generated test suites are 2 approaches Rob suggested in referenced CL, I've used in combination to reduce amount of auto-generated tests to be emitted and they helped a lot during implementation.

Also I didn't say, but will say now, I would prefer that such massive test files, such as this one and the ones in 115858, should be generated during the test rather than checked in.

I agree on this one, but when I've joined the project, it already had big auto-generated asm test file. I though it's logical and consistent to add another one to make it cover new instructions. That's it.

quasilyte commented 6 years ago

Also, I want to emphasize: If AVX-512 ISA is extended (and it almost certainly will), new instructions can be implemented as easily as running x86avxgen again. And test generator is ready for it. This was initial goal.

quasilyte commented 6 years ago

CC @TocarIP

quasilyte commented 6 years ago

I'll describe current situation to make this discussion easier. Just in case any details are missing from my previous responses.

I would like a proper discussion about how to test the assembler (and the compiler, if you like). Without a clear picture, the tests will become enormous, wildly inconsistent, messy, and difficult to maintain.

With x86, it was "almost" exhaustive auto-generated tests for both legacy and AVX instructions. amd64enc.s had some forms uncovered due to different reasons, one of them is special cases for some instructions. When errors in asm were discovered, tests were added to amd64enc_extra, to avoid modifications to auto-generated test suite. With AVX-512 I think auto-generated tests cover all angles, except opcode suffixes. If we are to generate opcode suffixes for each form, it will increase amount of auto generated tests by a measurable margin, while adding all combinations to amd64enc only takes around 10 lines.

Hand-written tests also catch regression bugs. These can be probably be separated. They are annoying to support in auto-generated tests because most of these tests have backwards-compatibility reasons that are not reflected in ISA/encoding or any other technical aspects. The're just there.

This is the current approach, we can discuss alternatives and improvements now, if the situation is clear.

mmcloughlin commented 5 years ago

Where is the code for avx512test? It's referenced in all the AVX-512 testdata files but I can't seem to find it.

https://github.com/golang/go/blob/50bd1c4d4eb4fac8ddeb5f063c099daccfb71b26/src/cmd/asm/internal/asm/testdata/avx512enc/avx512_ifma.s#L1

Likewise, was x86test was ever landed? https://golang.org/cl/18842

I don't personally see the need to generate these test files on the fly. I tend to have a preference for keeping the files in the repo, but with a CI job that confirms they are in sync with the generator (for example run go generate ./... and confirm no changes to the git repo). This also has the benefit of preventing hand edits.

However I think it is is problematic if the programs that generated them are unavailable or out of sync.

I don't have the same perspective as others here, so apologies if I'm speaking out of turn. It seems to me it would be great to get to the point where all x86 generated files in the Go repo can be reproduced from tools in golang.org/x/arch/x86. Looks like this might require:

Ideally, some form of https://golang.org/cl/104496 could be landed as well. The goal would be that all these code generators take their input from the same (canonical) database of Go x86 instructions.

Thoughts? I'm interested in contributing in these areas if I can be of use :)

pwaller commented 5 years ago

Ping @quasilyte - any chance of an update in response to @mmcloughlin? A few people are interested in picking up where you left off but it's unclear what the final state of things was.

quasilyte commented 5 years ago

Thanks for the ping. I thought I answered @mmcloughlin, but seems I was distracted and completely forgot about it. I'll open source avx512test in a few days.

mmcloughlin commented 5 years ago

Great, thanks @quasilyte! From my point of view it doesn't need to be clean at all, just throw up whatever you have on your machine :) The initial goal is just to get to the point where we can reproduce all these generated files, then we can consider any cleanup/refactor.

gopherbot commented 5 years ago

Change https://golang.org/cl/170863 mentions this issue: cmd/gopherbot: CC triaged issues to owners