From 8b20fd000d7e894865442134f9d6d197ac5dabed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandru=20Mo=C8=99oi?= Date: Tue, 12 Apr 2016 18:24:34 +0200 Subject: [PATCH] cmd/compile: transform some Phis into Or8. func f(a, b bool) bool { return a || b } is now a single instructions (excluding loading and unloading the arguments): v10 = ORB v11 v12 : AX Change-Id: Iff63399410cb46909f4318ea1c3f45a029f4aa5e Reviewed-on: https://go-review.googlesource.com/21872 TryBot-Result: Gobot Gobot Reviewed-by: Josh Bleecher Snyder --- src/cmd/compile/internal/ssa/compile.go | 3 +- src/cmd/compile/internal/ssa/phiopt.go | 57 ++++++++++++++----------- test/phiopt.go | 40 +++++++++++++++-- test/prove.go | 4 ++ 4 files changed, 74 insertions(+), 30 deletions(-) diff --git a/src/cmd/compile/internal/ssa/compile.go b/src/cmd/compile/internal/ssa/compile.go index a0b5ff71cf8..bc9c830ee95 100644 --- a/src/cmd/compile/internal/ssa/compile.go +++ b/src/cmd/compile/internal/ssa/compile.go @@ -289,8 +289,9 @@ var passOrder = [...]constraint{ {"opt", "nilcheckelim"}, // tighten should happen before lowering to avoid splitting naturally paired instructions such as CMP/SET {"tighten", "lower"}, - // cse, nilcheckelim, prove and loopbce share idom. + // cse, phiopt, nilcheckelim, prove and loopbce share idom. {"generic domtree", "generic cse"}, + {"generic domtree", "phiopt"}, {"generic domtree", "nilcheckelim"}, {"generic domtree", "prove"}, {"generic domtree", "loopbce"}, diff --git a/src/cmd/compile/internal/ssa/phiopt.go b/src/cmd/compile/internal/ssa/phiopt.go index 2d0a45733ad..4efd497bdb8 100644 --- a/src/cmd/compile/internal/ssa/phiopt.go +++ b/src/cmd/compile/internal/ssa/phiopt.go @@ -45,44 +45,51 @@ func phiopt(f *Func) { } // b0 is the if block giving the boolean value. - var reverse bool + // reverse is the predecessor from which the truth value comes. + var reverse int if b0.Succs[0] == pb0 && b0.Succs[1] == pb1 { - reverse = false + reverse = 0 } else if b0.Succs[0] == pb1 && b0.Succs[1] == pb0 { - reverse = true + reverse = 1 } else { b.Fatalf("invalid predecessors\n") } for _, v := range b.Values { - if v.Op != OpPhi || !v.Type.IsBoolean() || v.Args[0].Op != OpConstBool || v.Args[1].Op != OpConstBool { + if v.Op != OpPhi || !v.Type.IsBoolean() { continue } - ok, isCopy := false, false - if v.Args[0].AuxInt == 1 && v.Args[1].AuxInt == 0 { - ok, isCopy = true, !reverse - } else if v.Args[0].AuxInt == 0 && v.Args[1].AuxInt == 1 { - ok, isCopy = true, reverse - } - - // (Phi (ConstBool [x]) (ConstBool [x])) is already handled by opt / phielim. - - if ok && isCopy { - if f.pass.debug > 0 { - f.Config.Warnl(b.Line, "converted OpPhi to OpCopy") + // Replaces + // if a { x = true } else { x = false } with x = a + // and + // if a { x = false } else { x = true } with x = !a + if v.Args[0].Op == OpConstBool && v.Args[1].Op == OpConstBool { + if v.Args[reverse].AuxInt != v.Args[1-reverse].AuxInt { + ops := [2]Op{OpNot, OpCopy} + v.reset(ops[v.Args[reverse].AuxInt]) + v.AddArg(b0.Control) + if f.pass.debug > 0 { + f.Config.Warnl(b.Line, "converted OpPhi to %v", v.Op) + } + continue } - v.reset(OpCopy) - v.AddArg(b0.Control) - continue } - if ok && !isCopy { - if f.pass.debug > 0 { - f.Config.Warnl(b.Line, "converted OpPhi to OpNot") + + // Replaces + // if a { x = true } else { x = value } with x = a || value. + // Requires that value dominates x, meaning that regardless of a, + // value is always computed. This guarantees that the side effects + // of value are not seen if a is false. + if v.Args[reverse].Op == OpConstBool && v.Args[reverse].AuxInt == 1 { + if tmp := v.Args[1-reverse]; f.sdom.isAncestorEq(tmp.Block, b) { + v.reset(OpOr8) + v.SetArgs2(b0.Control, tmp) + if f.pass.debug > 0 { + f.Config.Warnl(b.Line, "converted OpPhi to %v", v.Op) + } + continue } - v.reset(OpNot) - v.AddArg(b0.Control) - continue } } } diff --git a/test/phiopt.go b/test/phiopt.go index 9b9b701124f..37caab0b513 100644 --- a/test/phiopt.go +++ b/test/phiopt.go @@ -1,8 +1,13 @@ // +build amd64 // errorcheck -0 -d=ssa/phiopt/debug=3 +// Copyright 2016 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 +//go:noinline func f0(a bool) bool { x := false if a { @@ -10,9 +15,10 @@ func f0(a bool) bool { } else { x = false } - return x // ERROR "converted OpPhi to OpCopy$" + return x // ERROR "converted OpPhi to Copy$" } +//go:noinline func f1(a bool) bool { x := false if a { @@ -20,23 +26,49 @@ func f1(a bool) bool { } else { x = true } - return x // ERROR "converted OpPhi to OpNot$" + return x // ERROR "converted OpPhi to Not$" } +//go:noinline func f2(a, b int) bool { x := true if a == b { x = false } - return x // ERROR "converted OpPhi to OpNot$" + return x // ERROR "converted OpPhi to Not$" } +//go:noinline func f3(a, b int) bool { x := false if a == b { x = true } - return x // ERROR "converted OpPhi to OpCopy$" + return x // ERROR "converted OpPhi to Copy$" +} + +//go:noinline +func f4(a, b bool) bool { + return a || b // ERROR "converted OpPhi to Or8$" +} + +//go:noinline +func f5(a int, b bool) bool { + x := b + if a == 0 { + x = true + } + return x // ERROR "converted OpPhi to Or8$" +} + +//go:noinline +func f6(a int, b bool) bool { + x := b + if a == 0 { + // f6 has side effects so the OpPhi should not be converted. + x = f6(a, b) + } + return x } func main() { diff --git a/test/prove.go b/test/prove.go index a78adf03dce..8bcc9ae6140 100644 --- a/test/prove.go +++ b/test/prove.go @@ -1,6 +1,10 @@ // +build amd64 // errorcheck -0 -d=ssa/prove/debug=3 +// Copyright 2016 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 import "math"