Skip to content

Commit

Permalink
cmd/compile: fix min/max builtin code generation
Browse files Browse the repository at this point in the history
Our large-function phi placement algorithm is incompatible with phi
opcodes already existing in the SSA representation. Instead, use simple
variable assignments and have the phi placement algorithm place the phis
we need for min/max.

Turns out the small-function phi placement algorithm doesn't have this
sensitivity, so this bug only occurs in large functions (>500 basic blocks).

Maybe we should document/check that no phis are present when we start
phi placement (regardless of size).  Leaving for a potential separate CL.

We should probably also fix the placement algorithm to handle existing
phis correctly.  But this CL is probably a lot smaller/safer than
messing with phi placement.

Fixes #60982

Change-Id: I59ba7f506c72b22bc1485099a335d96315ebef67
Reviewed-on: https://go-review.googlesource.com/c/go/+/505756
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
  • Loading branch information
randall77 committed Jun 24, 2023
1 parent ea927e5 commit a031f4e
Show file tree
Hide file tree
Showing 2 changed files with 2,030 additions and 3 deletions.
15 changes: 12 additions & 3 deletions src/cmd/compile/internal/ssagen/ssa.go
Expand Up @@ -949,6 +949,7 @@ var (
typVar = ssaMarker("typ")
okVar = ssaMarker("ok")
deferBitsVar = ssaMarker("deferBits")
ternaryVar = ssaMarker("ternary")
)

// startBlock sets the current block we're generating code in to b.
Expand Down Expand Up @@ -3622,18 +3623,26 @@ func (s *state) minMax(n *ir.CallExpr) *ssa.Value {
func (s *state) ternary(cond, x, y *ssa.Value) *ssa.Value {
bThen := s.f.NewBlock(ssa.BlockPlain)
bElse := s.f.NewBlock(ssa.BlockPlain)
bEnd := s.f.NewBlock(ssa.BlockPlain)

b := s.endBlock()
b.Kind = ssa.BlockIf
b.SetControl(cond)
b.AddEdgeTo(bThen)
b.AddEdgeTo(bElse)

s.startBlock(bThen)
s.vars[ternaryVar] = x
s.endBlock().AddEdgeTo(bEnd)

s.startBlock(bElse)
s.endBlock().AddEdgeTo(bThen)
s.vars[ternaryVar] = y
s.endBlock().AddEdgeTo(bEnd)

s.startBlock(bThen)
return s.newValue2(ssa.OpPhi, x.Type, x, y)
s.startBlock(bEnd)
r := s.variable(ternaryVar, x.Type)
delete(s.vars, ternaryVar)
return r
}

// condBranch evaluates the boolean expression cond and branches to yes
Expand Down

0 comments on commit a031f4e

Please sign in to comment.