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

Jump to bottom

cmd/compile: internal compiler error: "schedule does not include all values in block" on linux-s390x-ibm #38356

Closed
bcmills opened this issue Apr 10, 2020 · 5 comments
Closed

cmd/compile: internal compiler error: "schedule does not include all values in block" on linux-s390x-ibm #38356

bcmills opened this issue Apr 10, 2020 · 5 comments
Labels
arch-s390x Issues solely affecting the s390x architecture. FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.15

Comments

@bcmills
Copy link
Contributor

bcmills commented Apr 10, 2020

See also #37246.

https://build.golang.org/log/37189dee0a49af1e6e538c7e05d0e6a402f5a210

# golang.org/x/perf/internal/stats
internal/stats/utest.go:127:13: internal compiler error: 'MannWhitneyUTest': schedule does not include all values in block b80

goroutine 21 [running]:
runtime/debug.Stack(0x9c51a0, 0xc00000e018, 0x0)
	/data/golang/workdir/go/src/runtime/debug/stack.go:24 +0x98
cmd/compile/internal/gc.Fatalf(0xc000b96080, 0x36, 0xc000a7dba0, 0x2, 0x2)
	/data/golang/workdir/go/src/cmd/compile/internal/gc/subr.go:193 +0x170
cmd/compile/internal/gc.(*ssafn).Fatalf(0xc000a667e0, 0xd0007f0d1, 0x870026, 0x30, 0xc000a5b3e0, 0x1, 0x1)
	/data/golang/workdir/go/src/cmd/compile/internal/gc/ssa.go:6857 +0x184
cmd/compile/internal/ssa.(*Func).Fatalf(...)
	/data/golang/workdir/go/src/cmd/compile/internal/ssa/func.go:625
cmd/compile/internal/ssa.schedule(0xc0004d8420)
	/data/golang/workdir/go/src/cmd/compile/internal/ssa/schedule.go:293 +0x1194
cmd/compile/internal/ssa.Compile(0xc0004d8420)
	/data/golang/workdir/go/src/cmd/compile/internal/ssa/compile.go:93 +0x946
cmd/compile/internal/gc.buildssa(0xc0000d8420, 0x3, 0x0)
	/data/golang/workdir/go/src/cmd/compile/internal/gc/ssa.go:460 +0x93c
cmd/compile/internal/gc.compileSSA(0xc0000d8420, 0x3)
	/data/golang/workdir/go/src/cmd/compile/internal/gc/pgen.go:296 +0x40
cmd/compile/internal/gc.compileFunctions.func2(0xc000a83140, 0xc000498240, 0x3)
	/data/golang/workdir/go/src/cmd/compile/internal/gc/pgen.go:361 +0x40
created by cmd/compile/internal/gc.compileFunctions
	/data/golang/workdir/go/src/cmd/compile/internal/gc/pgen.go:359 +0x116

CC @mundaym @randall77
@bcmills bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. arch-s390x Issues solely affecting the s390x architecture. labels Apr 10, 2020
@bcmills bcmills added this to the Backlog milestone Apr 10, 2020
@mundaym
Copy link
Member

mundaym commented Apr 12, 2020

Steps to reproduce (I used the compiler at revision 83bfe3b and perf at revision 9c9101d):

git clone https://go.googlesource.com/perf
cd perf/internal/stats
GOOS=linux GOARCH=s390x go test -c

Output:

# golang.org/x/perf/internal/stats [golang.org/x/perf/internal/stats.test]
./utest.go:127:13: internal compiler error: 'MannWhitneyUTest': schedule does not include all values in block b80

goroutine 29 [running]:
runtime/debug.Stack(0xd6ff40, 0xc00000e018, 0x0)
	/home/michael/Development/go/master/src/runtime/debug/stack.go:24 +0x9d
cmd/compile/internal/gc.Fatalf(0xc0003f9d40, 0x36, 0xc000a98380, 0x2, 0x2)
	/home/michael/Development/go/master/src/cmd/compile/internal/gc/subr.go:193 +0x1aa
cmd/compile/internal/gc.(*ssafn).Fatalf(0xc0009cf680, 0x7f0d10000000d, 0xc1c4a1, 0x30, 0xc0003f6d60, 0x1, 0x1)
	/home/michael/Development/go/master/src/cmd/compile/internal/gc/ssa.go:6843 +0x19b
cmd/compile/internal/ssa.(*Func).Fatalf(...)
	/home/michael/Development/go/master/src/cmd/compile/internal/ssa/func.go:625
cmd/compile/internal/ssa.schedule(0xc000b634a0)
	/home/michael/Development/go/master/src/cmd/compile/internal/ssa/schedule.go:293 +0x1abd
cmd/compile/internal/ssa.Compile(0xc000b634a0)
	/home/michael/Development/go/master/src/cmd/compile/internal/ssa/compile.go:93 +0x9bb
cmd/compile/internal/gc.buildssa(0xc0000e89a0, 0x3, 0x0)
	/home/michael/Development/go/master/src/cmd/compile/internal/gc/ssa.go:460 +0xcca
cmd/compile/internal/gc.compileSSA(0xc0000e89a0, 0x3)
	/home/michael/Development/go/master/src/cmd/compile/internal/gc/pgen.go:296 +0x5d
cmd/compile/internal/gc.compileFunctions.func2(0xc000b97260, 0xc000017da0, 0x3)
	/home/michael/Development/go/master/src/cmd/compile/internal/gc/pgen.go:361 +0x49
created by cmd/compile/internal/gc.compileFunctions
	/home/michael/Development/go/master/src/cmd/compile/internal/gc/pgen.go:359 +0x128

@mundaym
Copy link
Member

mundaym commented Apr 12, 2020

I printed a bit more information. The scheduler seems to assume the Select0 and Select1 ops are always in the same block as their argument, which is probably necessary for register allocation, however that invariant appears to be broken in this case:

schedule does not include all values in block b80
  value not found: v724 = Select1 <flags> v616 (block b80)
    argument 0: v616 = FSUB <float64,flags> v282 v403 (block b70)

Floating point ops generating flags was added in CL 209160. I think the oversight in that CL is that the new rules do not force the Select1 to be in the same block as the original value:

(LTDBR (Select0 x:(F(ADD|SUB) _ _)))   -> (Select1 x)
(LTEBR (Select0 x:(F(ADDS|SUBS) _ _))) -> (Select1 x)

The solution is probably to replace these rules with:

(LTDBR (Select0 x:(F(ADD|SUB) _ _)))   && b == x.Block -> (Select1 x)
(LTEBR (Select0 x:(F(ADDS|SUBS) _ _))) && b == x.Block -> (Select1 x)

Alternatively we could use @x.Block, but we probably don't want to move the block that the flags are generated in for this optimization.

cc @ruixin-bao

@gopherbot
Copy link

Change https://golang.org/cl/227879 mentions this issue: cmd/compile: fix incorrect block for s390x Select1 op

@mundaym mundaym self-assigned this Apr 12, 2020
@mundaym mundaym modified the milestones: Backlog, Go1.15 Apr 12, 2020
@ruixin-bao
Copy link
Contributor

Thank you for looking into this issue.

Steps to reproduce (I used the compiler at revision 83bfe3b and perf at revision 9c9101d):

I am able to reproduce it as well.

The solution is probably to replace these rules with:
(LTDBR (Select0 x:(F(ADD|SUB) _ _))) && b == x.Block -> (Select1 x)
(LTEBR (Select0 x:(F(ADDS|SUBS) _ _))) && b == x.Block -> (Select1 x)

It looks like so. FWIW, I have tested the CL you sent and can confirm it fixes the problem.

@gopherbot
Copy link

Change https://golang.org/cl/237118 mentions this issue: cmd/compile: always tighten and de-duplicate tuple selectors

gopherbot pushed a commit that referenced this issue Jun 10, 2020
@mundaym
cmd/compile: always tighten and de-duplicate tuple selectors
ac743de 
The scheduler assumes two special invariants that apply to tuple
selectors (Select0 and Select1 ops):

  1. There is only one tuple selector of each type per generator.
  2. Tuple selectors and generators reside in the same block.

Prior to this CL the assumption was that these invariants would
only be broken by the CSE pass. The CSE pass therefore contained
code to move and de-duplicate selectors to fix these invariants.

However it is also possible to write relatively basic optimization
rules that cause these invariants to be broken. For example:

  (A (Select0 (B))) -> (Select1 (B))

This rule could result in the newly added selector (Select1) being
in a different block to the tuple generator (see issue #38356). It
could also result in duplicate selectors if this rule matches
multiple times for the same tuple generator (see issue #39472).

The CSE pass will 'fix' these invariants. However it will only do
so when optimizations are enabled (since disabling optimizations
disables the CSE pass).

This CL moves the CSE tuple selector fixup code into its own pass
and makes it mandatory even when optimizations are disabled. This
allows tuple selectors to be treated like normal ops for most of
the compilation pipeline until after the new pass has run, at which
point we need to be careful to maintain the invariant again.

Fixes #39472.

Change-Id: Ia3f79e09d9c65ac95f897ce37e967ee1258a080b
Reviewed-on: https://go-review.googlesource.com/c/go/+/237118
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
@golang golang locked and limited conversation to collaborators Jun 9, 2021
@rsc rsc unassigned mundaym Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
arch-s390x Issues solely affecting the s390x architecture. FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.15
Development

No branches or pull requests
4 participants
@bcmills @gopherbot @mundaym @ruixin-bao