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

test: all codegen tests are skipped #48247

Closed
mundaym opened this issue Sep 8, 2021 · 8 comments
Closed

test: all codegen tests are skipped #48247

mundaym opened this issue Sep 8, 2021 · 8 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.

Comments

@mundaym
Copy link
Member

mundaym commented Sep 8, 2021

Apologies if this is a duplicate issue, I couldn't find an appropriate one. I was surprised by this behaviour so I wanted to make sure it was flagged even if it is behaving as expected during the register ABI transition.

I think this behaviour was introduced by CL 320609. The issue can be worked around (albeit with a test failure on amd64) by applying the following patch:

diff --git a/test/run.go b/test/run.go
index 76621d9242..256dd04992 100644
--- a/test/run.go
+++ b/test/run.go
@@ -771,7 +771,7 @@ func (t *test) run() {
                t.initExpectFail(hasGFlag)
 
                switch tool {
-               case Build, Run:
+               case Build, Run, AsmCheck:
                        // ok; handled in goGcflags
 
                case Compile:

Is it worth skipping any failing tests explicitly and then re-enabling the rest? Most of the tests pass fine.

What version of Go are you using (go version)?

$ go version go version devel go1.18-50c69cc3a9 Wed Sep 8 06:59:06 2021 +0000 linux/amd64

Does this issue reproduce with the latest release?

Not tried.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env

What did you do?

cd test
../bin/go run run.go -v codegen

What did you expect to see?

Tests executing.

What did you see instead?

Tests skipped:

excl    codegen/addrcalc.go
ok      codegen/addrcalc.go     0.000s
excl    codegen/alloc.go
ok      codegen/alloc.go        0.000s
excl    codegen/arithmetic.go
ok      codegen/arithmetic.go   0.000s
excl    codegen/bitfield.go
ok      codegen/bitfield.go     0.000s
excl    codegen/bits.go
ok      codegen/bits.go 0.000s
excl    codegen/bool.go
ok      codegen/bool.go 0.000s
excl    codegen/clobberdead.go
ok      codegen/clobberdead.go  0.000s
excl    codegen/clobberdeadreg.go
ok      codegen/clobberdeadreg.go       0.000s
excl    codegen/compare_and_branch.go
ok      codegen/compare_and_branch.go   0.000s
excl    codegen/comparisons.go
ok      codegen/comparisons.go  0.000s
excl    codegen/condmove.go
ok      codegen/condmove.go     0.000s
excl    codegen/copy.go
ok      codegen/copy.go 0.000s
excl    codegen/floats.go
ok      codegen/floats.go       0.000s
excl    codegen/fuse.go
ok      codegen/fuse.go 0.000s
excl    codegen/issue22703.go
ok      codegen/issue22703.go   0.000s
excl    codegen/issue25378.go
ok      codegen/issue25378.go   0.000s
excl    codegen/issue31618.go
ok      codegen/issue31618.go   0.000s
excl    codegen/issue33580.go
ok      codegen/issue33580.go   0.000s
excl    codegen/issue38554.go
ok      codegen/issue38554.go   0.000s
excl    codegen/issue42610.go
ok      codegen/issue42610.go   0.000s
excl    codegen/issue48054.go
ok      codegen/issue48054.go   0.000s
excl    codegen/logic.go
ok      codegen/logic.go        0.000s
excl    codegen/mapaccess.go
ok      codegen/mapaccess.go    0.000s
excl    codegen/maps.go
ok      codegen/maps.go 0.000s
excl    codegen/math.go
ok      codegen/math.go 0.000s
excl    codegen/mathbits.go
ok      codegen/mathbits.go     0.000s
excl    codegen/memcombine.go
ok      codegen/memcombine.go   0.000s
excl    codegen/memops.go
ok      codegen/memops.go       0.000s
excl    codegen/noextend.go
ok      codegen/noextend.go     0.000s
excl    codegen/race.go
ok      codegen/race.go 0.000s
excl    codegen/regabi_regalloc.go
ok      codegen/regabi_regalloc.go      0.000s
excl    codegen/retpoline.go
ok      codegen/retpoline.go    0.000s
excl    codegen/rotate.go
ok      codegen/rotate.go       0.000s
excl    codegen/select.go
ok      codegen/select.go       0.000s
excl    codegen/shift.go
ok      codegen/shift.go        0.000s
excl    codegen/shortcircuit.go
ok      codegen/shortcircuit.go 0.000s
excl    codegen/slices.go
ok      codegen/slices.go       0.000s
excl    codegen/smallintiface.go
ok      codegen/smallintiface.go        0.000s
excl    codegen/spectre.go
ok      codegen/spectre.go      0.000s
excl    codegen/stack.go
ok      codegen/stack.go        0.000s
excl    codegen/strings.go
ok      codegen/strings.go      0.000s
excl    codegen/structs.go
ok      codegen/structs.go      0.000s
excl    codegen/switch.go
ok      codegen/switch.go       0.000s
excl    codegen/zerosize.go
ok      codegen/zerosize.go     0.000s
@mundaym
Copy link
Member Author

mundaym commented Sep 8, 2021

cc @mdempsky

@gopherbot
Copy link

Change https://golang.org/cl/348390 mentions this issue: test/codegen: remove broken riscv64 test

@gopherbot
Copy link

Change https://golang.org/cl/348391 mentions this issue: test/codegen: fix compilation of bitfield tests

@gopherbot
Copy link

Change https://golang.org/cl/348392 mentions this issue: test/codegen: fix package name for test case

@mundaym
Copy link
Member Author

mundaym commented Sep 8, 2021

Once the three CLs above are merged I believe the codegen tests can be re-enabled with the patch in the issue description.

gopherbot pushed a commit that referenced this issue Sep 8, 2021
This test is not executed by default (see #48247) and does not
actually pass. It was added in CL 346689. The code generation
changes made in that CL only change how instructions are assembled,
they do not actually affect the output of the compiler. This test
is unfortunately therefore invalid and will never pass.

Updates #48247.

Change-Id: I0c807e4a111336e5a097fe4e3af2805f9932a87f
Reviewed-on: https://go-review.googlesource.com/c/go/+/348390
Trust: Michael Munday <mike.munday@lowrisc.org>
Run-TryBot: Michael Munday <mike.munday@lowrisc.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Sep 8, 2021
The codegen tests are currently skipped (see #48247) and the
bitfield tests do not actually compile due to a duplicate function
name (sbfiz5) added in CL 267602. Renaming the function fixes the
issue.

Updates #48247.

Change-Id: I626fd5ef13732dc358e73ace9ddcc4cbb6ae5b21
Reviewed-on: https://go-review.googlesource.com/c/go/+/348391
Trust: Michael Munday <mike.munday@lowrisc.org>
Run-TryBot: Michael Munday <mike.munday@lowrisc.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
gopherbot pushed a commit that referenced this issue Sep 8, 2021
The codegen tests are currently skipped (see #48247). The test
added in CL 346050 did not compile because it was in the main
package but did not contain a main function. Changing the package
to 'codegen' fixes the issue.

Updates #48247.

Change-Id: I0a0eaca8e6a7d7b335606d2c76a204ac0c12e6d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/348392
Trust: Michael Munday <mike.munday@lowrisc.org>
Run-TryBot: Michael Munday <mike.munday@lowrisc.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
@mdempsky
Copy link
Member

mdempsky commented Sep 8, 2021

Thanks, I forgot that AsmCheck was skipped for -G=3. I'll send a fix.

@mdempsky mdempsky self-assigned this Sep 8, 2021
@mdempsky mdempsky added the NeedsFix The path to resolution is known, but the work has not been done. label Sep 8, 2021
@gopherbot
Copy link

Change https://golang.org/cl/348671 mentions this issue: test: re-enable AsmCheck tests for types2-based frontends

@gopherbot
Copy link

Change https://golang.org/cl/348670 mentions this issue: cmd/compile: extrapolate $GOROOT in unified IR

gopherbot pushed a commit that referenced this issue Sep 9, 2021
This ensures that diagnostics for files within $GOROOT continue to be
reported using their full filepath, rather than the abbreviated
filepath. Notably, this is necessary for test/run.go, which has tests
that expect to see the full filepath.

Updates #48247.

Change-Id: I440e2c6dd6109ca059d81cee49e476bba805d703
Reviewed-on: https://go-review.googlesource.com/c/go/+/348670
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants