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: recent regression in ppc64x builders: internal compiler error: pointer in non-pointer register g #25504

Closed
laboger opened this issue May 22, 2018 · 5 comments
Closed

cmd/compile: recent regression in ppc64x builders: internal compiler error: pointer in non-pointer register g #25504

laboger opened this issue May 22, 2018 · 5 comments
Labels
FrozenDueToAge release-blocker
Milestone
Go1.11

Comments

@laboger
Copy link
Contributor

laboger commented May 22, 2018

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

tip

Does this issue reproduce with the latest release?

yes

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

ppc64 & ppc64le

A regression was introduced earlier today which then hid 2 more regressions introduced after which affect the ppc64 & ppc64le builders. The first two were fixed, now seeing this:

--- FAIL: TestCoverpkgAllRuntime (7.07s)
go_test.go:6004: running testgo [test -coverpkg=all x]
go_test.go:6004: standard output:
go_test.go:6004: FAIL x [build failed]

go_test.go:6004: standard error:
go_test.go:6004: # runtime
	../../runtime/proc.go:4101: internal compiler error: pointer in non-pointer register g
	
	goroutine 6 [running]:
	runtime/debug.Stack(0x0, 0x0, 0x0)
		/home/boger/golang/really.plain/go/src/runtime/debug/stack.go:24 +0x94
	cmd/compile/internal/gc.Fatalf(0x81eeca, 0x22, 0xc0049be8e0, 0x1, 0x1)
		/home/boger/golang/really.plain/go/src/cmd/compile/internal/gc/subr.go:182 +0x1c8
	cmd/compile/internal/gc.(*ssafn).Fatalf(0xc0077d8e40, 0x10060920000003e, 0x81eeca, 0x22, 0xc0049be8e0, 0x1, 0x1)
		/home/boger/golang/really.plain/go/src/cmd/compile/internal/gc/ssa.go:5537 +0x64
	cmd/compile/internal/ssa.(*Value).Fatalf(0xc004065ee0, 0x81eeca, 0x22, 0xc0049be8e0, 0x1, 0x1)
		/home/boger/golang/really.plain/go/src/cmd/compile/internal/ssa/value.go:296 +0x7c
	cmd/compile/internal/gc.(*Liveness).regEffects.func1(0x0, 0xc004065ee0, 0x7e6101, 0xc000000020)
		/home/boger/golang/really.plain/go/src/cmd/compile/internal/gc/plive.go:446 +0x20c
	cmd/compile/internal/gc.(*Liveness).regEffects(0xc006d845a0, 0xc004066500, 0xffffffff)
		/home/boger/golang/really.plain/go/src/cmd/compile/internal/gc/plive.go:462 +0xcc
	cmd/compile/internal/gc.(*Liveness).prologue(0xc006d845a0)
		/home/boger/golang/really.plain/go/src/cmd/compile/internal/gc/plive.go:814 +0x108
	cmd/compile/internal/gc.liveness(0xc0077d8e40, 0xc007882580, 0xd)
		/home/boger/golang/really.plain/go/src/cmd/compile/internal/gc/plive.go:1714 +0x80
	cmd/compile/internal/gc.genssa(0xc007882580, 0xc006892070)
		/home/boger/golang/really.plain/go/src/cmd/compile/internal/gc/ssa.go:4774 +0x78
	cmd/compile/internal/gc.compileSSA(0xc0024deb00, 0x2)
		/home/boger/golang/really.plain/go/src/cmd/compile/internal/gc/pgen.go:267 +0x180
	cmd/compile/internal/gc.compileFunctions.func2(0xc0046f1080, 0xc00087bb60, 0x2)
		/home/boger/golang/really.plain/go/src/cmd/compile/internal/gc/pgen.go:309 +0x40
	created by cmd/compile/internal/gc.compileFunctions
		/home/boger/golang/really.plain/go/src/cmd/compile/internal/gc/pgen.go:307 +0x11c
	
	
go_test.go:6004: go [test -coverpkg=all x] failed unexpectedly: exit status 2

FAIL
FAIL cmd/go 139.178s

Assuming this is related to the reg changes that have been just added? @aclements
@aclements
Copy link
Member

Assuming this is related to the reg changes that have been just added?

It is. @dr2chase has been digging into the cause of this, which is quite peculiar. For some reason we're spilling the g register to the stack and then reloading it back into the g register, which is completely ridiculous and quite possible unsafe (e.g., if there's a setg between the spill and the reload, this would clobber it's effect).

I might temporarily ignore the g register in liveness analysis, just to get the build green again.

/cc @khr

@aclements aclements added this to the Go1.11 milestone May 22, 2018
@gopherbot
Copy link

Change https://golang.org/cl/114081 mentions this issue: cmd/compile: ignore g register in liveness analysis

gopherbot pushed a commit that referenced this issue May 22, 2018
@aclements
cmd/compile: ignore g register in liveness analysis
1329009 
In rare circumstances that we don't yet fully understand, the g
register can be spilled to the stack and then reloaded. If this
happens, liveness analysis sees a pointer load into a
non-general-purpose register and panics.

We should fix the root cause of this, but fix the build for now by
ignoring pointer loads into the g register.

For #25504.

Change-Id: I0dfee1af9750c8e9157c7637280cdf07118ef2ca
Reviewed-on: https://go-review.googlesource.com/114081
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/114087 mentions this issue: Revert "cmd/compile: ignore g register in liveness analysis"

@dr2chase
Copy link
Contributor

I think I understand what's going on, though not completely.
The two moving parts are in runtime/proc.go.
On line 4079: _g_ := getg()
which end up producing this assembly language (line number is off-by-one, sigh):

 v1004 	00524 (4080)	MOVW	R6, 3628(R5)
 v1074 	00525 (4080)	MOVD	g, ""._g_-16(R1)
 v1032 	00526 (4081)	MOVD	g, R8
 v1008 	00527 (4081)	MOVD	48(R8), R9

I.e., _g_ gets register-allocated to the g register.
And then, a few blocks later:

 v1210 	00572 (4106)	CALL	"".runqempty(SB)

which clobbers all the registers, including the one with _g_ in it (which is g).
Because _g_ is live in the same loop, it is subsequently reloaded (into g) to be an input to the phi function at the top of the loop. It appears that the register allocator is just following orders.

The SSA dump after trim:

b119: ← b123 b57
v1003 (4080) = Phi <mem> v998 v961
v1004 (+4080) = MOVWstore <mem> [3628] v1260 v1255 v1003
v1005 (+4080) = GetG <*g> v1004 : g (_g_[*g])
v1074 (+4080) = StoreReg <*g> v1005 : _g_[*g]
v1032 (4081) = Copy <*g> v1005 : R8
v1008 (4081) = MOVDload <*m> [48] v1032 v1004 : R9
v1011 (4081) = MOVDload <puintptr> [208] v1008 v1004 : R9
v1385 (4081) = CMPconst <flags> [0] v1011
NE v1385 → b128 b106 (likely) (line 4081)
b128: ← b119
v1024 (224) = MOVDaddr <*struct { Count [20]uint32; Pos [60]uint32; NumStmt [20]uint16 }> {"".GoCover_68_643932633661383162326666} v3 : R10
v1025 (+224) = MOVWstore <mem> [20] v1024 v1255 v1004
v1026 (224) = Convert <unsafe.Pointer> v1011 v1025 : R9 (~r0[*p])
v1033 (+4081) = MOVWload <int32> [8] v1026 v1025 : R9
v1365 (4081) = CMPW <flags> v1033 v1312
LT v1365 → b129 b126 (likely) (line 4081)
b129: ← b128
v1041 (+4081) = MOVWstore <mem> [3812] v1260 v1255 v1025
v1045 (+4083) = MOVDload <*m> [48] v1032 v1041 : R9
v1048 (4083) = MOVDload <puintptr> [208] v1045 v1041 : R9 (pp[puintptr])
v1052 (+224) = MOVWstore <mem> [20] v1024 v1255 v1041
v1053 (224) = Convert <unsafe.Pointer> v1048 v1052 : R9 (~r0[*p])
v1060 (4083) = MOVWstore <mem> [12] v1053 v1255 v1052
Plain → b125 (line 4083)
b125: ← b129 b136
v1152 (4099) = Phi <mem> v1060 v1355
v1153 (+4099) = MOVWstore <mem> [3632] v1260 v1255 v1152
v1155 (+4100) = ADDconst <int32> [-1] v1312 : R9 (i[int32])
v584 (?) = MOVDconst <*p> [0] : R11
Plain → b137 (line 4100)
b139: ← b144 b147
v1332 (4110) = Phi <*p> v1331 v608 : R11 (runnablePs[*p])
v1347 (4114) = Phi <mem> v1198 v1348
v1308 (+4100) = ADDconst <int32> [-1] v1156 : R9 (i[int32])
Plain → b137 (line 4100)
b137: ← b125 b139
v1156 (4100) = Phi <int32> v1155 v1308 : R9 (i[int32])
v1331 (4110) = Phi <*p> v584 v1332 : R11 (runnablePs[*p], p[*p])
v1346 (4114) = Phi <mem> v1153 v1347
v1016 (4110) = StoreReg <*p> v1331 : p[*p]
v899 (+4100) = CMPWconst <flags> [0] v1156
GE v899 → b138 b140 (likely) (line 4100)
b138: ← b137
v1163 (+4100) = MOVWstore <mem> [3832] v1260 v1255 v1346
v1167 (4101) = MOVWreg <int> v1156 : R12
v670 (+4101) = MOVDload <**p> v831 v1163 : R14
v675 (+4101) = MOVDload <int> [8] v831 v1163 : R15
v778 (4101) = CMPU <flags> v1167 v675
LT v778 → b143 b142 (likely) (line 4101)
b143: ← b138
v1179 (+4102) = MOVDload <*m> [48] v1032 v1163 : R15
v625 (4101) = SLDconst <int> [3] v1167 : R12
v1173 (4101) = ADD <**p> v670 v625 : R12
v1175 (4101) = MOVDload <*p> v1173 v1163 : R12 (runnablePs[*p], p[*p])
v1182 (+4102) = MOVDload <puintptr> [208] v1179 v1163 : R14 (pp[puintptr])
v1186 (+224) = MOVWstore <mem> [20] v1024 v1255 v1163
v1187 (+224) = Convert <unsafe.Pointer> v1182 v1186 : R14 (~r0[*p])
v795 (+4102) = CMP <flags> v1187 v1175
EQ v795 → b144 b145 (likely) (line 4102)
b144: ← b143
v1198 (+4102) = MOVWstore <mem> [3840] v1260 v1255 v1186
Plain → b139 (line +4103)
b145: ← b143
v1406 (4100) = StoreReg <int32> v1156 : i[int32]
v1023 (4101) = StoreReg <*p> v1175 : p[*p]
v1204 (+4105) = MOVWstore <mem> [3836] v1260 v1255 v1186
v1208 (4105) = MOVWstorezero <mem> [12] v1175 v1204
v1209 (+4106) = MOVDstore <mem> [32] v2 v1175 v1208
v1210 (4106) = CALLstatic <mem> {"".runqempty} [16] v1209
v1212 (4106) = MOVBZload <bool> [40] v2 v1210 : R3
v1373 (4106) = CMPWconst <flags> [0] v1212
NE v1373 → b146 b149 (unlikely) (line 4106)
b149: ← b145
v1072 (4108) = MOVDaddr <*struct { Count [1237]uint32; Pos [3711]uint32; NumStmt [1237]uint16 }> {"".GoCover_60_643932633661383162326666} v3 : R3
v1064 (4108) = MOVDconst <uint32> [1] : R4
v1228 (+4108) = MOVWstore <mem> [3848] v1072 v1064 v1210
v1237 (+4621) = MOVWstore <mem> [4412] v1072 v1064 v1228
v1393 (4622) = MOVDaddr <*schedt> {"".sched} v3 : R5
v1240 (+4622) = MOVDload <muintptr> [24] v1393 v1237 : R6 (mp[muintptr])
v1151 (241) = MOVDaddr <*struct { Count [20]uint32; Pos [60]uint32; NumStmt [20]uint16 }> {"".GoCover_68_643932633661383162326666} v3 : R7
v1245 (+241) = MOVWstore <mem> [28] v1151 v1064 v1237
v1246 (+241) = Convert <unsafe.Pointer> v1240 v1245 : R6 (mp[*m], ~r0[*m], m[*m], ~r0[*m])
v828 (+4623) = CMPconst <flags> [0] v1246
NE v828 → b150 b154 (line 4623)
b150: ← b149
v1257 (+4623) = MOVWstore <mem> [4420] v1072 v1064 v1245
v1261 (+4624) = MOVDload <muintptr> [336] v1246 v1257 : R8
v1264 (4624) = MOVDstore <mem> [24] v1393 v1261 v1257
v1267 (+4625) = MOVWload <int32> [32] v1393 v1264 : R8
v1268 (4625) = ADDconst <int32> [-1] v1267 : R8
v1271 (4625) = MOVWstore <mem> [32] v1393 v1268 v1264
Plain → b154 (line 4625)
b154: ← b149 b150
v1276 (4627) = Phi <mem> v1245 v1271
v1277 (+4627) = MOVWstore <mem> [4416] v1072 v1064 v1276
v1286 (+244) = MOVWstore <mem> [32] v1151 v1064 v1277
v1380 (4109) = LoadReg <*p> v1023 : R8
v1231 (4109) = ADDconst <*muintptr> [64] v1380 : R9 (mp[*muintptr])
v1290 (244) = LoweredNilCheck <void> v1231 v1286
v1288 (244) = Convert <muintptr> v1246 v1286 : R6
v1291 (244) = MOVDstore <mem> [64] v1380 v1288 v1286
v1301 (+227) = MOVWstore <mem> [24] v1151 v1064 v1291
v1295 (+4110) = ADDconst <*puintptr> [16] v1380 : R6 (pp[*puintptr])
v1304 (227) = LoweredNilCheck <void> v1295 v1301
v993 (227) = LoadReg <*p> v1016 : R6
v1303 (+227) = Convert <puintptr> v993 v1301 : R6
v1305 (227) = MOVDstore <mem> [16] v1380 v1303 v1301
Plain → b147 (line +4111)
b147: ← b146 b154
v1333 (4110) = Phi <*p> v642 v1380 : R8 (runnablePs[*p])
v1348 (4114) = Phi <mem> v1222 v1305
v618 (?) = MOVDaddr <*[]*p> {"".allp} v3 : R3
v616 (?) = MOVDaddr <*schedt> {"".sched} v3 : R4
v611 (?) = MOVDaddr <*struct { Count [1237]uint32; Pos [3711]uint32; NumStmt [1237]uint16 }> {"".GoCover_60_643932633661383162326666} v3 : R5
v587 (?) = MOVDconst <uint32> [1] : R6
v595 (4114) = LoadReg <int32> v6 : R7
v602 (4100) = LoadReg <int32> v1406 : R9
v598 (?) = MOVDaddr <*struct { Count [20]uint32; Pos [60]uint32; NumStmt [20]uint16 }> {"".GoCover_68_643932633661383162326666} v3 : R10
v605 (4102) = LoadReg <*g> v1074 : g
v608 (4110) = Copy <*p> v1333 : R11
v619 (4102) = Copy <*g> v605 : R8
Plain → b139 (line 4100)
b146: ← b145
v113 (4106) = MOVDaddr <*struct { Count [1237]uint32; Pos [3711]uint32; NumStmt [1237]uint16 }> {"".GoCover_60_643932633661383162326666} v3 : R3
v128 (4106) = MOVDconst <uint32> [1] : R4
v1219 (+4106) = MOVWstore <mem> [3844] v113 v128 v1210
v130 (4107) = LoadReg <*p> v1023 : R5
v1221 (+4107) = MOVDstore <mem> [32] v2 v130 v1219
v1222 (4107) = CALLstatic <mem> {"".pidleput} [8] v1221
v514 (?) = MOVDaddr <*struct { Count [1237]uint32; Pos [3711]uint32; NumStmt [1237]uint16 }> {"".GoCover_60_643932633661383162326666} v3 : R3
v512 (?) = MOVDconst <uint32> [1] : R4
v509 (?) = MOVDaddr <*schedt> {"".sched} v3 : R5
v637 (?) = MOVDaddr <*struct { Count [20]uint32; Pos [60]uint32; NumStmt [20]uint16 }> {"".GoCover_68_643932633661383162326666} v3 : R7
v642 (4110) = LoadReg <*p> v1016 : R8
Plain → b147 (line 4107)
b140: ← b137
v1314 (+4114) = MOVWstore <mem> [3636] v1260 v1255 v1346
v987 (4114) = MOVDaddr <*randomOrder> {"".stealOrder} v3 : R3
v1317 (4114) = MOVDstore <mem> [32] v2 v987 v1314
v1320 (4114) = MOVWstore <mem> [40] v2 v1312 v1317
v1321 (4114) = CALLstatic <mem> {"".(*randomOrder).reset} [16] v1320
v1403 (4116) = MOVDaddr <*int32> {"".gomaxprocs} v3 : R3
v971 (4116) = LoadReg <int32> v6 : R4
v1326 (+4116) = LoweredAtomicStore32 <mem> v1403 v971 v1321
v1328 (4117) = VarDef <mem> {~r1} v1326
v164 (4117) = LoadReg <*p> v1016 : R3
v1329 (+4117) = MOVDstore <mem> {~r1} v2 v164 v1328
Ret v1329 (line +4117)
b126: ← b106 b128
v1065 (4084) = Phi <mem> v1004 v1025
v1066 (+4084) = MOVWstore <mem> [3816] v1260 v1255 v1065
v1070 (+4086) = MOVDload <*m> [48] v1032 v1066 : R9
v1073 (4086) = MOVDload <puintptr> [208] v1070 v1066 : R9
v1062 (4086) = CMPconst <flags> [0] v1073
NE v1062 → b132 b131 (line 4086)
b132: ← b126
v1080 (+4086) = MOVWstore <mem> [3824] v1260 v1255 v1066
v1084 (4087) = MOVDload <*m> [48] v1032 v1080 : R9
v1087 (+4087) = MOVDload <puintptr> [208] v1084 v1080 : R9 (pp[puintptr])
v1091 (+224) = MOVWstore <mem> [20] v1024 v1255 v1080
v1092 (224) = Convert <unsafe.Pointer> v1087 v1091 : R9 (~r0[*p])
v1100 (4087) = MOVDstorezero <mem> [64] v1092 v1091
Plain → b131 (line 4087)
b131: ← b126 b132
v1105 (4089) = Phi <mem> v1066 v1100
v1106 (+4089) = MOVWstore <mem> [3820] v1260 v1255 v1105
v1110 (+4089) = MOVDload <*m> [48] v1032 v1106 : R9
v1113 (4089) = MOVDstorezero <mem> [208] v1110 v1106
v1116 (+4090) = MOVDload <*m> [48] v1032 v1113 : R9
v1119 (4090) = MOVDstorezero <mem> [344] v1116 v1113
v636 (+4091) = MOVDload <int> [8] v831 v1119 : R9
v1142 (+4091) = MOVDload <**p> v831 v1119 : R11
v1398 (4091) = CMPUconst <flags> [0] v636
GT v1398 → b133 b134 (likely) (line 4091)
b133: ← b131
v1129 (4091) = MOVDload <*p> v1142 v1119 : R3 (p[*p])
v1132 (+4092) = MOVDstorezero <mem> [64] v1129 v1119
v1135 (+4093) = MOVWstorezero <mem> [12] v1129 v1132
v1136 (+4094) = MOVDstore <mem> [32] v2 v1129 v1135
v1137 (4094) = CALLstatic <mem> {"".acquirep} [8] v1136
v167 (4095) = MOVDaddr <*struct { lock mutex; lockOwner *g; enabled bool; shutdown bool; headerWritten bool; footerWritten bool; shutdownSema uint32; seqStart uint64; ticksStart int64; ticksEnd int64; timeStart int64; timeEnd int64; seqGC uint64; reading traceBufPtr; empty traceBufPtr; fullHead traceBufPtr; fullTail traceBufPtr; reader guintptr; stackTab traceStackTable; stringsLock mutex; strings map[string]uint64; stringSeq uint64; markWorkerLabels [3]uint64; bufLock mutex; buf traceBufPtr }> {"".trace} v3 : R3
v1140 (+4095) = MOVBZload <bool> [16] v167 v1137 : R3
v1391 (+4095) = CMPWconst <flags> [0] v1140
NE v1391 → b135 b136 (unlikely) (line 4095)
b136: ← b133 b135
v1355 (4099) = Phi <mem> v1137 v1147
v566 (?) = MOVDaddr <*[]*p> {"".allp} v3 : R3
v559 (?) = MOVDaddr <*schedt> {"".sched} v3 : R4
v552 (?) = MOVDaddr <*struct { Count [1237]uint32; Pos [3711]uint32; NumStmt [1237]uint16 }> {"".GoCover_60_643932633661383162326666} v3 : R5
v543 (?) = MOVDconst <uint32> [1] : R6
v518 (4100) = LoadReg <int32> v6 : R7
v563 (4102) = LoadReg <*g> v1074 : R8
v572 (?) = MOVDaddr <*struct { Count [20]uint32; Pos [60]uint32; NumStmt [20]uint16 }> {"".GoCover_68_643932633661383162326666} v3 : R10
v578 (4102) = Copy <*g> v563 : g
Plain → b125 (line +4099)
b135: ← b133
v139 (4095) = MOVDaddr <*struct { Count [1237]uint32; Pos [3711]uint32; NumStmt [1237]uint16 }> {"".GoCover_60_643932633661383162326666} v3 : R3
v188 (4095) = MOVDconst <uint32> [1] : R4
v1146 (+4095) = MOVWstore <mem> [3828] v139 v188 v1137
v1147 (+4096) = CALLstatic <mem> {"".traceGoStart} v1146
Plain → b136 (line 4096)

@gopherbot
Copy link

Change https://golang.org/cl/114695 mentions this issue: cmd/compile: do not allow regalloc to LoadReg G register

gopherbot pushed a commit that referenced this issue May 31, 2018
@aclements
Revert "cmd/compile: ignore g register in liveness analysis"
33cd4fb 
This reverts commit ea200340702cf3ccfac7c5db1f11bb65c80971c7 now
that CL 114695 fixed the root cause of #25504.

Change-Id: If437fc832983bd8793bde28ce0e2e64436a0596c
Reviewed-on: https://go-review.googlesource.com/114087
Reviewed-by: David Chase <drchase@google.com>
@golang golang locked and limited conversation to collaborators May 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge release-blocker
Projects
None yet
Milestone
Go1.11
Development

No branches or pull requests
4 participants
@dr2chase @aclements @gopherbot @laboger