cmd/compile: fix SCCP propagation into jump tables

We can't delete all the outgoing edges and then add one back in, because
then we've lost the argument of any phi at the target. Instead, move
the important target to the front of the list and delete the rest.

This normally isn't a problem, because there is never normally a phi
at the target of a jump table. But this isn't quite true when in race
build mode, because there is a phi of the result of a bunch of raceread
calls.

The reason this happens is that each case is written like this (where e
is the runtime.eface we're switching on):

if e.type == $type.int32 {
   m = raceread(e.data, m1)
}
m2 = phi(m1, m)
if e.type == $type.int32 {
   .. do case ..
   goto blah
}

so that if e.type is not $type.int32, it falls through to the default
case. This default case will have a memory phi for all the (jumped around
and not actually called) raceread calls.

If we instead did it like

if e.type == $type.int32 {
  raceread(e.data)
  .. do case ..
  goto blah
}

That would paper over this bug, as it is the only way to construct
a jump table whose target is a block with a phi in it. (Yet.)

But we'll fix the underlying bug in this CL. Maybe we can do the
rewrite mentioned above later.  (It is an optimization for -race mode,
which isn't particularly important.)

Fixes #64606

Change-Id: I6f6e3c90eb1e2638112920ee2e5b6581cef04ea4
Reviewed-on: https://go-review.googlesource.com/c/go/+/548356
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This commit is contained in:
Keith Randall 2023-12-07 11:09:29 -08:00
parent dca2ef2361
commit 788a227759
4 changed files with 54 additions and 5 deletions

View file

@ -297,6 +297,8 @@ func (b *Block) removePred(i int) {
// removeSucc removes the ith output edge from b.
// It is the responsibility of the caller to remove
// the corresponding predecessor edge.
// Note that this potentially reorders successors of b, so it
// must be used very carefully.
func (b *Block) removeSucc(i int) {
n := len(b.Succs) - 1
if i != n {
@ -323,6 +325,19 @@ func (b *Block) swapSuccessors() {
b.Likely *= -1
}
// Swaps b.Succs[x] and b.Succs[y].
func (b *Block) swapSuccessorsByIdx(x, y int) {
if x == y {
return
}
ex := b.Succs[x]
ey := b.Succs[y]
b.Succs[x] = ey
b.Succs[y] = ex
ex.b.Preds[ex.i].i = y
ey.b.Preds[ey.i].i = x
}
// removePhiArg removes the ith arg from phi.
// It must be called after calling b.removePred(i) to
// adjust the corresponding phi value of the block:
@ -339,7 +354,7 @@ func (b *Block) swapSuccessors() {
func (b *Block) removePhiArg(phi *Value, i int) {
n := len(b.Preds)
if numPhiArgs := len(phi.Args); numPhiArgs-1 != n {
b.Fatalf("inconsistent state, num predecessors: %d, num phi args: %d", n, numPhiArgs)
b.Fatalf("inconsistent state for %v, num predecessors: %d, num phi args: %d", phi, n, numPhiArgs)
}
phi.Args[i].Uses--
phi.Args[i] = phi.Args[n]

View file

@ -312,6 +312,8 @@ func deadcode(f *Func) {
// removeEdge removes the i'th outgoing edge from b (and
// the corresponding incoming edge from b.Succs[i].b).
// Note that this potentially reorders successors of b, so it
// must be used very carefully.
func (b *Block) removeEdge(i int) {
e := b.Succs[i]
c := e.b

View file

@ -533,12 +533,12 @@ func rewireSuccessor(block *Block, constVal *Value) bool {
block.ResetControls()
return true
case BlockJumpTable:
// Remove everything but the known taken branch.
idx := int(constVal.AuxInt)
targetBlock := block.Succs[idx].b
for len(block.Succs) > 0 {
block.removeEdge(0)
block.swapSuccessorsByIdx(0, idx)
for len(block.Succs) > 1 {
block.removeEdge(1)
}
block.AddEdgeTo(targetBlock)
block.Kind = BlockPlain
block.Likely = BranchUnknown
block.ResetControls()

View file

@ -0,0 +1,32 @@
// build -race
//go:build race
// Copyright 2023 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
package main
func main() {
var o any = uint64(5)
switch o.(type) {
case int:
goto ret
case int8:
goto ret
case int16:
goto ret
case int32:
goto ret
case int64:
goto ret
case float32:
goto ret
case float64:
goto ret
default:
goto ret
}
ret:
}