1
0
mirror of https://github.com/golang/go synced 2024-11-14 05:50:27 -07:00

cmd/compile: fix min/max builtin code generation

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>
This commit is contained in:
Keith Randall 2023-06-23 15:29:36 -07:00 committed by Keith Randall
parent ea927e560d
commit a031f4ef83
2 changed files with 2031 additions and 4 deletions

View File

@ -949,6 +949,7 @@ var (
typVar = ssaMarker("typ") typVar = ssaMarker("typ")
okVar = ssaMarker("ok") okVar = ssaMarker("ok")
deferBitsVar = ssaMarker("deferBits") deferBitsVar = ssaMarker("deferBits")
ternaryVar = ssaMarker("ternary")
) )
// startBlock sets the current block we're generating code in to b. // startBlock sets the current block we're generating code in to b.
@ -3622,6 +3623,7 @@ func (s *state) minMax(n *ir.CallExpr) *ssa.Value {
func (s *state) ternary(cond, x, y *ssa.Value) *ssa.Value { func (s *state) ternary(cond, x, y *ssa.Value) *ssa.Value {
bThen := s.f.NewBlock(ssa.BlockPlain) bThen := s.f.NewBlock(ssa.BlockPlain)
bElse := s.f.NewBlock(ssa.BlockPlain) bElse := s.f.NewBlock(ssa.BlockPlain)
bEnd := s.f.NewBlock(ssa.BlockPlain)
b := s.endBlock() b := s.endBlock()
b.Kind = ssa.BlockIf b.Kind = ssa.BlockIf
@ -3629,11 +3631,18 @@ func (s *state) ternary(cond, x, y *ssa.Value) *ssa.Value {
b.AddEdgeTo(bThen) b.AddEdgeTo(bThen)
b.AddEdgeTo(bElse) b.AddEdgeTo(bElse)
s.startBlock(bElse)
s.endBlock().AddEdgeTo(bThen)
s.startBlock(bThen) s.startBlock(bThen)
return s.newValue2(ssa.OpPhi, x.Type, x, y) s.vars[ternaryVar] = x
s.endBlock().AddEdgeTo(bEnd)
s.startBlock(bElse)
s.vars[ternaryVar] = y
s.endBlock().AddEdgeTo(bEnd)
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 // condBranch evaluates the boolean expression cond and branches to yes

2018
test/fixedbugs/issue60982.go Normal file

File diff suppressed because it is too large Load Diff