From cde5fd1c0f8c40804bfd942eec1e2d69bccf4e13 Mon Sep 17 00:00:00 2001 From: Josh Bleecher Snyder Date: Thu, 13 Aug 2020 12:39:04 -0700 Subject: cmd/compile: correct type of CvtBoolToUint8 values Fixes #40746 Change-Id: I539f07d1f958dacee87d846171a8889d03182d25 Reviewed-on: https://go-review.googlesource.com/c/go/+/248397 Run-TryBot: Josh Bleecher Snyder TryBot-Result: Gobot Gobot Reviewed-by: Keith Randall --- test/fixedbugs/issue40746.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 test/fixedbugs/issue40746.go (limited to 'test') diff --git a/test/fixedbugs/issue40746.go b/test/fixedbugs/issue40746.go new file mode 100644 index 0000000000..235282fd90 --- /dev/null +++ b/test/fixedbugs/issue40746.go @@ -0,0 +1,19 @@ +// compile + +// Copyright 2020 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 p + +func f(x byte, b bool) byte { + var c byte + if b { + c = 1 + } + + if int8(c) < 0 { + x++ + } + return x +} -- cgit v1.3-5-g9baa From 32a84c99e136ed5af0686dbedd31fd7dff40fb38 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Sat, 8 Aug 2020 07:58:04 -0700 Subject: cmd/compile: fix live variable computation for deferreturn Taking the live variable set from the last return point is problematic. See #40629 for details, but there may not be a return point, or it may be before the final defer. Additionally, keeping track of the last call as a *Value doesn't quite work. If it is dead-code eliminated, the storage for the Value is reused for some other random instruction. Its live variable information, if it is available at all, is wrong. Instead, just mark all the open-defer argument slots as live throughout the function. (They are already zero-initialized.) Fixes #40629 Change-Id: Ie456c7db3082d0de57eaa5234a0f32525a1cce13 Reviewed-on: https://go-review.googlesource.com/c/go/+/247522 Run-TryBot: Keith Randall TryBot-Result: Gobot Gobot Reviewed-by: Dan Scales --- src/cmd/compile/internal/gc/plive.go | 118 ++++++++--------------------------- src/cmd/compile/internal/gc/ssa.go | 19 +----- src/cmd/compile/internal/ssa/func.go | 11 +--- test/fixedbugs/issue40629.go | 69 ++++++++++++++++++++ 4 files changed, 99 insertions(+), 118 deletions(-) create mode 100644 test/fixedbugs/issue40629.go (limited to 'test') diff --git a/src/cmd/compile/internal/gc/plive.go b/src/cmd/compile/internal/gc/plive.go index b366c8a4a0..0cb2661997 100644 --- a/src/cmd/compile/internal/gc/plive.go +++ b/src/cmd/compile/internal/gc/plive.go @@ -140,24 +140,14 @@ type Liveness struct { regMaps []liveRegMask cache progeffectscache - - // These are only populated if open-coded defers are being used. - // List of vars/stack slots storing defer args - openDeferVars []openDeferVarInfo - // Map from defer arg OpVarDef to the block where the OpVarDef occurs. - openDeferVardefToBlockMap map[*Node]*ssa.Block - // Map of blocks that cannot reach a return or exit (panic) - nonReturnBlocks map[*ssa.Block]bool -} - -type openDeferVarInfo struct { - n *Node // Var/stack slot storing a defer arg - varsIndex int // Index of variable in lv.vars } // LivenessMap maps from *ssa.Value to LivenessIndex. type LivenessMap struct { vals map[ssa.ID]LivenessIndex + // The set of live, pointer-containing variables at the deferreturn + // call (only set when open-coded defers are used). + deferreturn LivenessIndex } func (m *LivenessMap) reset() { @@ -168,6 +158,7 @@ func (m *LivenessMap) reset() { delete(m.vals, k) } } + m.deferreturn = LivenessInvalid } func (m *LivenessMap) set(v *ssa.Value, i LivenessIndex) { @@ -542,7 +533,7 @@ func newliveness(fn *Node, f *ssa.Func, vars []*Node, idx map[*Node]int32, stkpt if cap(lc.be) >= f.NumBlocks() { lv.be = lc.be[:f.NumBlocks()] } - lv.livenessMap = LivenessMap{lc.livenessMap.vals} + lv.livenessMap = LivenessMap{vals: lc.livenessMap.vals, deferreturn: LivenessInvalid} lc.livenessMap.vals = nil } if lv.be == nil { @@ -893,58 +884,12 @@ func (lv *Liveness) hasStackMap(v *ssa.Value) bool { func (lv *Liveness) prologue() { lv.initcache() - if lv.fn.Func.HasDefer() && !lv.fn.Func.OpenCodedDeferDisallowed() { - lv.openDeferVardefToBlockMap = make(map[*Node]*ssa.Block) - for i, n := range lv.vars { - if n.Name.OpenDeferSlot() { - lv.openDeferVars = append(lv.openDeferVars, openDeferVarInfo{n: n, varsIndex: i}) - } - } - - // Find any blocks that cannot reach a return or a BlockExit - // (panic) -- these must be because of an infinite loop. - reachesRet := make(map[ssa.ID]bool) - blockList := make([]*ssa.Block, 0, 256) - - for _, b := range lv.f.Blocks { - if b.Kind == ssa.BlockRet || b.Kind == ssa.BlockRetJmp || b.Kind == ssa.BlockExit { - blockList = append(blockList, b) - } - } - - for len(blockList) > 0 { - b := blockList[0] - blockList = blockList[1:] - if reachesRet[b.ID] { - continue - } - reachesRet[b.ID] = true - for _, e := range b.Preds { - blockList = append(blockList, e.Block()) - } - } - - lv.nonReturnBlocks = make(map[*ssa.Block]bool) - for _, b := range lv.f.Blocks { - if !reachesRet[b.ID] { - lv.nonReturnBlocks[b] = true - //fmt.Println("No reach ret", lv.f.Name, b.ID, b.Kind) - } - } - } - for _, b := range lv.f.Blocks { be := lv.blockEffects(b) // Walk the block instructions backward and update the block // effects with the each prog effects. for j := len(b.Values) - 1; j >= 0; j-- { - if b.Values[j].Op == ssa.OpVarDef { - n := b.Values[j].Aux.(*Node) - if n.Name.OpenDeferSlot() { - lv.openDeferVardefToBlockMap[n] = b - } - } pos, e := lv.valueEffects(b.Values[j]) regUevar, regKill := lv.regEffects(b.Values[j]) if e&varkill != 0 { @@ -961,20 +906,6 @@ func (lv *Liveness) prologue() { } } -// markDeferVarsLive marks each variable storing an open-coded defer arg as -// specially live in block b if the variable definition dominates block b. -func (lv *Liveness) markDeferVarsLive(b *ssa.Block, newliveout *varRegVec) { - // Only force computation of dominators if we have a block where we need - // to specially mark defer args live. - sdom := lv.f.Sdom() - for _, info := range lv.openDeferVars { - defB := lv.openDeferVardefToBlockMap[info.n] - if sdom.IsAncestorEq(defB, b) { - newliveout.vars.Set(int32(info.varsIndex)) - } - } -} - // Solve the liveness dataflow equations. func (lv *Liveness) solve() { // These temporary bitvectors exist to avoid successive allocations and @@ -1018,23 +949,6 @@ func (lv *Liveness) solve() { } } - if lv.fn.Func.HasDefer() && !lv.fn.Func.OpenCodedDeferDisallowed() && - (b.Kind == ssa.BlockExit || lv.nonReturnBlocks[b]) { - // Open-coded defer args slots must be live - // everywhere in a function, since a panic can - // occur (almost) anywhere. Force all appropriate - // defer arg slots to be live in BlockExit (panic) - // blocks and in blocks that do not reach a return - // (because of infinite loop). - // - // We are assuming that the defer exit code at - // BlockReturn/BlockReturnJmp accesses all of the - // defer args (with pointers), and so keeps them - // live. This analysis may have to be adjusted if - // that changes (because of optimizations). - lv.markDeferVarsLive(b, &newliveout) - } - if !be.liveout.Eq(newliveout) { change = true be.liveout.Copy(newliveout) @@ -1087,6 +1001,17 @@ func (lv *Liveness) epilogue() { n.Name.SetNeedzero(true) livedefer.Set(int32(i)) } + if n.Name.OpenDeferSlot() { + // Open-coded defer args slots must be live + // everywhere in a function, since a panic can + // occur (almost) anywhere. Because it is live + // everywhere, it must be zeroed on entry. + livedefer.Set(int32(i)) + // It was already marked as Needzero when created. + if !n.Name.Needzero() { + Fatalf("all pointer-containing defer arg slots should have Needzero set") + } + } } } @@ -1188,6 +1113,17 @@ func (lv *Liveness) epilogue() { lv.compact(b) } + // If we have an open-coded deferreturn call, make a liveness map for it. + if lv.fn.Func.OpenCodedDeferDisallowed() { + lv.livenessMap.deferreturn = LivenessInvalid + } else { + lv.livenessMap.deferreturn = LivenessIndex{ + stackMapIndex: lv.stackMapSet.add(livedefer), + regMapIndex: 0, // entry regMap, containing no live registers + isUnsafePoint: false, + } + } + // Done compacting. Throw out the stack map set. lv.stackMaps = lv.stackMapSet.extractUniqe() lv.stackMapSet = bvecSet{} diff --git a/src/cmd/compile/internal/gc/ssa.go b/src/cmd/compile/internal/gc/ssa.go index d4d23a2956..5d0098b4e6 100644 --- a/src/cmd/compile/internal/gc/ssa.go +++ b/src/cmd/compile/internal/gc/ssa.go @@ -4318,12 +4318,6 @@ func (s *state) openDeferExit() { } } - if i == len(s.openDefers)-1 { - // Record the call of the first defer. This will be used - // to set liveness info for the deferreturn (which is also - // used for any location that causes a runtime panic) - s.f.LastDeferExit = call - } s.endBlock() s.startBlock(bEnd) } @@ -5807,11 +5801,6 @@ type SSAGenState struct { // wasm: The number of values on the WebAssembly stack. This is only used as a safeguard. OnWasmStackSkipped int - - // Liveness index for the first function call in the final defer exit code - // path that we generated. All defer functions and args should be live at - // this point. This will be used to set the liveness for the deferreturn. - lastDeferLiveness LivenessIndex } // Prog appends a new Prog. @@ -6056,12 +6045,6 @@ func genssa(f *ssa.Func, pp *Progs) { // instruction. s.pp.nextLive = s.livenessMap.Get(v) - // Remember the liveness index of the first defer call of - // the last defer exit - if v.Block.Func.LastDeferExit != nil && v == v.Block.Func.LastDeferExit { - s.lastDeferLiveness = s.pp.nextLive - } - // Special case for first line in function; move it to the start. if firstPos != src.NoXPos { s.SetPos(firstPos) @@ -6122,7 +6105,7 @@ func genssa(f *ssa.Func, pp *Progs) { // When doing open-coded defers, generate a disconnected call to // deferreturn and a return. This will be used to during panic // recovery to unwind the stack and return back to the runtime. - s.pp.nextLive = s.lastDeferLiveness + s.pp.nextLive = s.livenessMap.deferreturn gencallret(pp, Deferreturn) } diff --git a/src/cmd/compile/internal/ssa/func.go b/src/cmd/compile/internal/ssa/func.go index 7cf72a8e37..4b9189fb3e 100644 --- a/src/cmd/compile/internal/ssa/func.go +++ b/src/cmd/compile/internal/ssa/func.go @@ -33,15 +33,8 @@ type Func struct { Blocks []*Block // unordered set of all basic blocks (note: not indexable by ID) Entry *Block // the entry basic block - // If we are using open-coded defers, this is the first call to a deferred - // function in the final defer exit sequence that we generated. This call - // should be after all defer statements, and will have all args, etc. of - // all defer calls as live. The liveness info of this call will be used - // for the deferreturn/ret segment generated for functions with open-coded - // defers. - LastDeferExit *Value - bid idAlloc // block ID allocator - vid idAlloc // value ID allocator + bid idAlloc // block ID allocator + vid idAlloc // value ID allocator // Given an environment variable used for debug hash match, // what file (if any) receives the yes/no logging? diff --git a/test/fixedbugs/issue40629.go b/test/fixedbugs/issue40629.go new file mode 100644 index 0000000000..c6ef408f49 --- /dev/null +++ b/test/fixedbugs/issue40629.go @@ -0,0 +1,69 @@ +// run + +// Copyright 2020 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 "fmt" + +const N = 40 + +func main() { + var x [N]int // stack-allocated memory + for i := range x { + x[i] = 0x999 + } + + // This defer checks to see if x is uncorrupted. + defer func(p *[N]int) { + recover() + for i := range p { + if p[i] != 0x999 { + for j := range p { + fmt.Printf("p[%d]=0x%x\n", j, p[j]) + } + panic("corrupted stack variable") + } + } + }(&x) + + // This defer starts a new goroutine, which will (hopefully) + // overwrite x on the garbage stack. + defer func() { + c := make(chan bool) + go func() { + useStack(1000) + c <- true + }() + <-c + + }() + + // This defer causes a stack copy. + // The old stack is now garbage. + defer func() { + useStack(1000) + }() + + // Trigger a segfault. + *g = 0 + + // Make the return statement unreachable. + // That makes the stack map at the deferreturn call empty. + // In particular, the argument to the first defer is not + // marked as a pointer, so it doesn't get adjusted + // during the stack copy. + for { + } +} + +var g *int64 + +func useStack(n int) { + if n == 0 { + return + } + useStack(n - 1) +} -- cgit v1.3-5-g9baa From f71444955a4c0962abb334a8f39438466c57a4db Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Fri, 14 Aug 2020 14:39:19 -0700 Subject: test: add test case that caused gccgo undefined symbol reference For #40252 Change-Id: Ie23d2789ca9b4b9081adb39ab64c80c412ad58ce Reviewed-on: https://go-review.googlesource.com/c/go/+/248637 Run-TryBot: Ian Lance Taylor Reviewed-by: Cherry Zhang TryBot-Result: Gobot Gobot --- test/fixedbugs/issue40252.dir/a.go | 14 ++++++++++++++ test/fixedbugs/issue40252.dir/main.go | 16 ++++++++++++++++ test/fixedbugs/issue40252.go | 8 ++++++++ 3 files changed, 38 insertions(+) create mode 100644 test/fixedbugs/issue40252.dir/a.go create mode 100644 test/fixedbugs/issue40252.dir/main.go create mode 100644 test/fixedbugs/issue40252.go (limited to 'test') diff --git a/test/fixedbugs/issue40252.dir/a.go b/test/fixedbugs/issue40252.dir/a.go new file mode 100644 index 0000000000..5519e9331a --- /dev/null +++ b/test/fixedbugs/issue40252.dir/a.go @@ -0,0 +1,14 @@ +// Copyright 2020 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 a + +type I interface { + Func() +} + +func Call() { + f := I.Func + f(nil) +} diff --git a/test/fixedbugs/issue40252.dir/main.go b/test/fixedbugs/issue40252.dir/main.go new file mode 100644 index 0000000000..93f5b70624 --- /dev/null +++ b/test/fixedbugs/issue40252.dir/main.go @@ -0,0 +1,16 @@ +// Copyright 2020 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 "./a" + +func main() { + defer func() { + if recover() == nil { + panic("expected nil pointer dereference") + } + }() + a.Call() +} diff --git a/test/fixedbugs/issue40252.go b/test/fixedbugs/issue40252.go new file mode 100644 index 0000000000..9be4e665d2 --- /dev/null +++ b/test/fixedbugs/issue40252.go @@ -0,0 +1,8 @@ +// rundir + +// Copyright 2020 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. + +// gccgo got an undefined symbol reference when inlining a method expression. +package ignored -- cgit v1.3-5-g9baa From 6072d6ee3e173bb46370dabc158e2cb25a6f4877 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Fri, 10 Jul 2020 09:56:00 -0700 Subject: test: add a test case that gccgo fails to compile Change-Id: If36394e059cdae49834d26ad4ffdd3092a72a0b8 Reviewed-on: https://go-review.googlesource.com/c/go/+/241997 Run-TryBot: Ian Lance Taylor TryBot-Result: Gobot Gobot Reviewed-by: Cherry Zhang --- test/fixedbugs/bug509.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 test/fixedbugs/bug509.go (limited to 'test') diff --git a/test/fixedbugs/bug509.go b/test/fixedbugs/bug509.go new file mode 100644 index 0000000000..df6ed61f89 --- /dev/null +++ b/test/fixedbugs/bug509.go @@ -0,0 +1,30 @@ +// compile + +// Copyright 2020 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. + +// Gccgo mishandles a couple of alias cases. + +package p + +type S struct{} + +func (*S) M() {} + +type I interface { + M() +} + +type A = *S + +var V1 I +var _ = V1.(*S) +var _ = V1.(A) + +func F() { + var v I + v = (*S)(nil) + v = A(nil) + _ = v +} -- cgit v1.3-5-g9baa From 12d40adac46b5c771247a789205f7893bfd808b2 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Fri, 10 Jul 2020 12:21:41 -0700 Subject: test: add test for conversion of untyped bool to interface gccgo miscompiled this case. Updates #40152 Change-Id: I8448c155e802e39d8fc7cda4930ce62cb6363ce5 Reviewed-on: https://go-review.googlesource.com/c/go/+/242000 Run-TryBot: Ian Lance Taylor TryBot-Result: Gobot Gobot Reviewed-by: Emmanuel Odeke Reviewed-by: Cherry Zhang --- test/fixedbugs/issue40152.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 test/fixedbugs/issue40152.go (limited to 'test') diff --git a/test/fixedbugs/issue40152.go b/test/fixedbugs/issue40152.go new file mode 100644 index 0000000000..1cb68e9914 --- /dev/null +++ b/test/fixedbugs/issue40152.go @@ -0,0 +1,21 @@ +// run + +// Copyright 2020 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. + +// Gccgo mishandles converting an untyped boolean to an interface type. + +package main + +func t(args ...interface{}) bool { + x := true + return x == args[0] +} + +func main() { + r := t("x" == "x" && "y" == "y") + if !r { + panic(r) + } +} -- cgit v1.3-5-g9baa From 0031fa80a3c6685e44e84533edbae0dad0eb0395 Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Thu, 7 May 2020 00:35:28 +0700 Subject: cmd/compile: another fix initializing blank fields in struct literal CL 230121 fixed the bug that struct literal blank fields type array/struct can not be initialized. But it still misses some cases when an expression causes "candiscard(value)" return false. When these happen, we recursively call fixedlit with "var_" set to "_", and hit the bug again. To fix it, just making splitnode return "nblank" whenever "var_" is "nblank". Fixes #38905 Change-Id: I281941b388acbd551a4d8ca1a235477f8d26fb6e Reviewed-on: https://go-review.googlesource.com/c/go/+/232617 Run-TryBot: Cuong Manh Le TryBot-Result: Gobot Gobot Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/gc/sinit.go | 6 +++++- test/fixedbugs/issue38905.go | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 test/fixedbugs/issue38905.go (limited to 'test') diff --git a/src/cmd/compile/internal/gc/sinit.go b/src/cmd/compile/internal/gc/sinit.go index 4a2edc7d21..71ed558461 100644 --- a/src/cmd/compile/internal/gc/sinit.go +++ b/src/cmd/compile/internal/gc/sinit.go @@ -506,6 +506,7 @@ const ( // fixedlit handles struct, array, and slice literals. // TODO: expand documentation. func fixedlit(ctxt initContext, kind initKind, n *Node, var_ *Node, init *Nodes) { + isBlank := var_ == nblank var splitnode func(*Node) (a *Node, value *Node) switch n.Op { case OARRAYLIT, OSLICELIT: @@ -520,6 +521,9 @@ func fixedlit(ctxt initContext, kind initKind, n *Node, var_ *Node, init *Nodes) } a := nod(OINDEX, var_, nodintconst(k)) k++ + if isBlank { + a = nblank + } return a, r } case OSTRUCTLIT: @@ -527,7 +531,7 @@ func fixedlit(ctxt initContext, kind initKind, n *Node, var_ *Node, init *Nodes) if r.Op != OSTRUCTKEY { Fatalf("fixedlit: rhs not OSTRUCTKEY: %v", r) } - if r.Sym.IsBlank() { + if r.Sym.IsBlank() || isBlank { return nblank, r.Left } setlineno(r) diff --git a/test/fixedbugs/issue38905.go b/test/fixedbugs/issue38905.go new file mode 100644 index 0000000000..6f411b8605 --- /dev/null +++ b/test/fixedbugs/issue38905.go @@ -0,0 +1,18 @@ +// compile + +// Copyright 2020 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. + +// Make sure that literal value can be passed to struct +// blank field with expressions where candiscard(value) +// returns false, see #38905. + +package p + +type t struct{ _ u } +type u [10]struct{ f int } + +func f(x int) t { return t{u{{1 / x}, {1 % x}}} } +func g(p *int) t { return t{u{{*p}}} } +func h(s []int) t { return t{u{{s[0]}}} } -- cgit v1.3-5-g9baa From 82c45eb68187d7827bca392d528dbfa06607e3f0 Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Mon, 27 Jul 2020 12:08:56 +0700 Subject: cmd/compile: handle OCLOSURE/OCALLPART in mustHeapAlloc check Currently, generated struct wrapper for closure is not handled in mustHeapAlloc. That causes compiler crashes when the wrapper struct is too large for stack, and must be heap allocated instead. Fixes #39292 Change-Id: I14c1e591681d9d92317bb2396d6cf5207aa93e08 Reviewed-on: https://go-review.googlesource.com/c/go/+/244917 Run-TryBot: Cuong Manh Le TryBot-Result: Gobot Gobot Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/gc/closure.go | 2 +- src/cmd/compile/internal/gc/esc.go | 7 +++++++ test/fixedbugs/issue39292.go | 29 +++++++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 test/fixedbugs/issue39292.go (limited to 'test') diff --git a/src/cmd/compile/internal/gc/closure.go b/src/cmd/compile/internal/gc/closure.go index 3bb7bb9834..04fb7d5495 100644 --- a/src/cmd/compile/internal/gc/closure.go +++ b/src/cmd/compile/internal/gc/closure.go @@ -526,7 +526,7 @@ func walkpartialcall(n *Node, init *Nodes) *Node { // Create closure in the form of a composite literal. // For x.M with receiver (x) type T, the generated code looks like: // - // clos = &struct{F uintptr; R T}{M.T·f, x} + // clos = &struct{F uintptr; R T}{T.M·f, x} // // Like walkclosure above. diff --git a/src/cmd/compile/internal/gc/esc.go b/src/cmd/compile/internal/gc/esc.go index f3e9ab78ef..628953741a 100644 --- a/src/cmd/compile/internal/gc/esc.go +++ b/src/cmd/compile/internal/gc/esc.go @@ -187,6 +187,13 @@ func mustHeapAlloc(n *Node) bool { return true } + if n.Op == OCLOSURE && closureType(n).Size() >= maxImplicitStackVarSize { + return true + } + if n.Op == OCALLPART && partialCallType(n).Size() >= maxImplicitStackVarSize { + return true + } + if n.Op == OMAKESLICE && !isSmallMakeSlice(n) { return true } diff --git a/test/fixedbugs/issue39292.go b/test/fixedbugs/issue39292.go new file mode 100644 index 0000000000..5d6595c234 --- /dev/null +++ b/test/fixedbugs/issue39292.go @@ -0,0 +1,29 @@ +// errorcheck -0 -m -l + +// Copyright 2020 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 p + +type t [10000]*int + +func (t) f() { +} + +func x() { + x := t{}.f // ERROR "t literal.f escapes to heap" + x() +} + +func y() { + var i int // ERROR "moved to heap: i" + y := (&t{&i}).f // ERROR "\(&t literal\).f escapes to heap" "&t literal escapes to heap" + y() +} + +func z() { + var i int // ERROR "moved to heap: i" + z := t{&i}.f // ERROR "t literal.f escapes to heap" + z() +} -- cgit v1.3-5-g9baa From 948d324f7d8641a042da46c25417ebabd84e5e78 Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Wed, 12 Aug 2020 23:12:57 +0700 Subject: cmd/compile: add failing test case for #24305 Updates #24305 Change-Id: Ib0b093e33004a978467cdd1e77186798392d4eca Reviewed-on: https://go-review.googlesource.com/c/go/+/248217 Run-TryBot: Cuong Manh Le TryBot-Result: Gobot Gobot Reviewed-by: Matthew Dempsky --- test/escape5.go | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'test') diff --git a/test/escape5.go b/test/escape5.go index 061e57a069..2ed2023cd2 100644 --- a/test/escape5.go +++ b/test/escape5.go @@ -179,6 +179,13 @@ func _() { u.N() } +func fbad24305() { + // BAD u should not be heap allocated + var u U // ERROR "moved to heap: u" + (*U).M(&u) + (*U).N(&u) +} + // Issue 24730: taking address in a loop causes unnecessary escape type T24730 struct { x [64]byte -- cgit v1.3-5-g9baa From 69d34e2c6965f70fe1ead3e7e8ab45ada3267ebc Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Fri, 14 Aug 2020 21:30:04 -0700 Subject: test: bump array size in fixedbugs/issue39292.go The previous array length was large enough to exceed maxImplicitStackSize on 64-bit architectures, but not on 32-bit architectures. Fixes #40808. Change-Id: I69e9abb447454b2e7875ba503a0cb772e965ae31 Reviewed-on: https://go-review.googlesource.com/c/go/+/248680 Run-TryBot: Matthew Dempsky Reviewed-by: Cuong Manh Le TryBot-Result: Gobot Gobot --- test/fixedbugs/issue39292.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'test') diff --git a/test/fixedbugs/issue39292.go b/test/fixedbugs/issue39292.go index 5d6595c234..1be88653e9 100644 --- a/test/fixedbugs/issue39292.go +++ b/test/fixedbugs/issue39292.go @@ -6,7 +6,7 @@ package p -type t [10000]*int +type t [20000]*int func (t) f() { } -- cgit v1.3-5-g9baa From dc12d5b0f5e9c1cfec2a8eb6dd7ff3473c36d45c Mon Sep 17 00:00:00 2001 From: Tobias Klauser Date: Mon, 17 Aug 2020 11:28:26 +0200 Subject: all: add empty line between copyright header and package clause Makes sure the copyright notice is not interpreted as the package level godoc. Change-Id: I2afce7c9d620f19d51ec1438b1d0db1774b57146 Reviewed-on: https://go-review.googlesource.com/c/go/+/248760 Run-TryBot: Tobias Klauser TryBot-Result: Gobot Gobot Reviewed-by: Dave Cheney --- src/cmd/compile/internal/ssa/debug.go | 1 + src/cmd/compile/internal/ssa/passbm_test.go | 1 + src/cmd/go/internal/trace/trace.go | 1 + src/cmd/link/internal/benchmark/bench_test.go | 1 + src/cmd/link/internal/ld/errors.go | 1 + src/runtime/closure_test.go | 1 + src/runtime/map_benchmark_test.go | 1 + src/runtime/slice_test.go | 1 + src/sync/cond_test.go | 1 + test/fixedbugs/issue15281.go | 1 + 10 files changed, 10 insertions(+) (limited to 'test') diff --git a/src/cmd/compile/internal/ssa/debug.go b/src/cmd/compile/internal/ssa/debug.go index 13fe67cbca..6353f72897 100644 --- a/src/cmd/compile/internal/ssa/debug.go +++ b/src/cmd/compile/internal/ssa/debug.go @@ -1,6 +1,7 @@ // Copyright 2017 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 ssa import ( diff --git a/src/cmd/compile/internal/ssa/passbm_test.go b/src/cmd/compile/internal/ssa/passbm_test.go index eefdbb8722..3fd3eb579b 100644 --- a/src/cmd/compile/internal/ssa/passbm_test.go +++ b/src/cmd/compile/internal/ssa/passbm_test.go @@ -1,6 +1,7 @@ // Copyright 2015 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 ssa import ( diff --git a/src/cmd/go/internal/trace/trace.go b/src/cmd/go/internal/trace/trace.go index 7cb7636a34..c8fac92c9f 100644 --- a/src/cmd/go/internal/trace/trace.go +++ b/src/cmd/go/internal/trace/trace.go @@ -1,6 +1,7 @@ // Copyright 2020 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 trace import ( diff --git a/src/cmd/link/internal/benchmark/bench_test.go b/src/cmd/link/internal/benchmark/bench_test.go index d8ec717c7c..419dc55724 100644 --- a/src/cmd/link/internal/benchmark/bench_test.go +++ b/src/cmd/link/internal/benchmark/bench_test.go @@ -1,6 +1,7 @@ // Copyright 2020 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 benchmark import ( diff --git a/src/cmd/link/internal/ld/errors.go b/src/cmd/link/internal/ld/errors.go index c5ce097fde..d6e8ff236d 100644 --- a/src/cmd/link/internal/ld/errors.go +++ b/src/cmd/link/internal/ld/errors.go @@ -1,6 +1,7 @@ // Copyright 2020 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 ld import ( diff --git a/src/runtime/closure_test.go b/src/runtime/closure_test.go index ea65fbd5f5..741c932eab 100644 --- a/src/runtime/closure_test.go +++ b/src/runtime/closure_test.go @@ -1,6 +1,7 @@ // Copyright 2011 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 runtime_test import "testing" diff --git a/src/runtime/map_benchmark_test.go b/src/runtime/map_benchmark_test.go index 893cb6c5b6..d0becc9ddb 100644 --- a/src/runtime/map_benchmark_test.go +++ b/src/runtime/map_benchmark_test.go @@ -1,6 +1,7 @@ // Copyright 2013 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 runtime_test import ( diff --git a/src/runtime/slice_test.go b/src/runtime/slice_test.go index e963a43dd3..cd2bc26d1e 100644 --- a/src/runtime/slice_test.go +++ b/src/runtime/slice_test.go @@ -1,6 +1,7 @@ // Copyright 2011 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 runtime_test import ( diff --git a/src/sync/cond_test.go b/src/sync/cond_test.go index 9d0d9adc74..859cae59bc 100644 --- a/src/sync/cond_test.go +++ b/src/sync/cond_test.go @@ -1,6 +1,7 @@ // Copyright 2011 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 sync_test import ( diff --git a/test/fixedbugs/issue15281.go b/test/fixedbugs/issue15281.go index 187c96f218..390867c848 100644 --- a/test/fixedbugs/issue15281.go +++ b/test/fixedbugs/issue15281.go @@ -3,6 +3,7 @@ // 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 "runtime" -- cgit v1.3-5-g9baa From e30fbe3757d09a22988835835c41233df7c6cd00 Mon Sep 17 00:00:00 2001 From: Junchen Li Date: Fri, 10 Jul 2020 11:39:23 +0800 Subject: cmd/compile: optimize unsigned comparisons to 0 There are some architecture-independent rules in #21439, since an unsigned integer >= 0 is always true and < 0 is always false. This CL adds these optimizations to generic rules. Updates #21439 Change-Id: Iec7e3040b761ecb1e60908f764815fdd9bc62495 Reviewed-on: https://go-review.googlesource.com/c/go/+/246617 Reviewed-by: Keith Randall Run-TryBot: Keith Randall TryBot-Result: Gobot Gobot --- src/cmd/compile/internal/ssa/gen/generic.rules | 4 ++ src/cmd/compile/internal/ssa/rewritegeneric.go | 80 ++++++++++++++++++++++++++ test/codegen/comparisons.go | 17 ++++++ 3 files changed, 101 insertions(+) (limited to 'test') diff --git a/src/cmd/compile/internal/ssa/gen/generic.rules b/src/cmd/compile/internal/ssa/gen/generic.rules index ed5bfc81fd..2d39d27226 100644 --- a/src/cmd/compile/internal/ssa/gen/generic.rules +++ b/src/cmd/compile/internal/ssa/gen/generic.rules @@ -545,6 +545,10 @@ (Or(64|32|16|8) x (Or(64|32|16|8) x y)) => (Or(64|32|16|8) x y) (Xor(64|32|16|8) x (Xor(64|32|16|8) x y)) => y +// Unsigned comparisons to zero. +(Less(64U|32U|16U|8U) _ (Const(64|32|16|8) [0])) => (ConstBool [false]) +(Leq(64U|32U|16U|8U) (Const(64|32|16|8) [0]) _) => (ConstBool [true]) + // Ands clear bits. Ors set bits. // If a subsequent Or will set all the bits // that an And cleared, we can skip the And. diff --git a/src/cmd/compile/internal/ssa/rewritegeneric.go b/src/cmd/compile/internal/ssa/rewritegeneric.go index 9f4e1b95bd..68e49f46f3 100644 --- a/src/cmd/compile/internal/ssa/rewritegeneric.go +++ b/src/cmd/compile/internal/ssa/rewritegeneric.go @@ -9701,6 +9701,16 @@ func rewriteValuegeneric_OpLeq16U(v *Value) bool { v.AuxInt = boolToAuxInt(uint16(c) <= uint16(d)) return true } + // match: (Leq16U (Const16 [0]) _) + // result: (ConstBool [true]) + for { + if v_0.Op != OpConst16 || auxIntToInt16(v_0.AuxInt) != 0 { + break + } + v.reset(OpConstBool) + v.AuxInt = boolToAuxInt(true) + return true + } return false } func rewriteValuegeneric_OpLeq32(v *Value) bool { @@ -9805,6 +9815,16 @@ func rewriteValuegeneric_OpLeq32U(v *Value) bool { v.AuxInt = boolToAuxInt(uint32(c) <= uint32(d)) return true } + // match: (Leq32U (Const32 [0]) _) + // result: (ConstBool [true]) + for { + if v_0.Op != OpConst32 || auxIntToInt32(v_0.AuxInt) != 0 { + break + } + v.reset(OpConstBool) + v.AuxInt = boolToAuxInt(true) + return true + } return false } func rewriteValuegeneric_OpLeq64(v *Value) bool { @@ -9909,6 +9929,16 @@ func rewriteValuegeneric_OpLeq64U(v *Value) bool { v.AuxInt = boolToAuxInt(uint64(c) <= uint64(d)) return true } + // match: (Leq64U (Const64 [0]) _) + // result: (ConstBool [true]) + for { + if v_0.Op != OpConst64 || auxIntToInt64(v_0.AuxInt) != 0 { + break + } + v.reset(OpConstBool) + v.AuxInt = boolToAuxInt(true) + return true + } return false } func rewriteValuegeneric_OpLeq8(v *Value) bool { @@ -9993,6 +10023,16 @@ func rewriteValuegeneric_OpLeq8U(v *Value) bool { v.AuxInt = boolToAuxInt(uint8(c) <= uint8(d)) return true } + // match: (Leq8U (Const8 [0]) _) + // result: (ConstBool [true]) + for { + if v_0.Op != OpConst8 || auxIntToInt8(v_0.AuxInt) != 0 { + break + } + v.reset(OpConstBool) + v.AuxInt = boolToAuxInt(true) + return true + } return false } func rewriteValuegeneric_OpLess16(v *Value) bool { @@ -10033,6 +10073,16 @@ func rewriteValuegeneric_OpLess16U(v *Value) bool { v.AuxInt = boolToAuxInt(uint16(c) < uint16(d)) return true } + // match: (Less16U _ (Const16 [0])) + // result: (ConstBool [false]) + for { + if v_1.Op != OpConst16 || auxIntToInt16(v_1.AuxInt) != 0 { + break + } + v.reset(OpConstBool) + v.AuxInt = boolToAuxInt(false) + return true + } return false } func rewriteValuegeneric_OpLess32(v *Value) bool { @@ -10093,6 +10143,16 @@ func rewriteValuegeneric_OpLess32U(v *Value) bool { v.AuxInt = boolToAuxInt(uint32(c) < uint32(d)) return true } + // match: (Less32U _ (Const32 [0])) + // result: (ConstBool [false]) + for { + if v_1.Op != OpConst32 || auxIntToInt32(v_1.AuxInt) != 0 { + break + } + v.reset(OpConstBool) + v.AuxInt = boolToAuxInt(false) + return true + } return false } func rewriteValuegeneric_OpLess64(v *Value) bool { @@ -10153,6 +10213,16 @@ func rewriteValuegeneric_OpLess64U(v *Value) bool { v.AuxInt = boolToAuxInt(uint64(c) < uint64(d)) return true } + // match: (Less64U _ (Const64 [0])) + // result: (ConstBool [false]) + for { + if v_1.Op != OpConst64 || auxIntToInt64(v_1.AuxInt) != 0 { + break + } + v.reset(OpConstBool) + v.AuxInt = boolToAuxInt(false) + return true + } return false } func rewriteValuegeneric_OpLess8(v *Value) bool { @@ -10193,6 +10263,16 @@ func rewriteValuegeneric_OpLess8U(v *Value) bool { v.AuxInt = boolToAuxInt(uint8(c) < uint8(d)) return true } + // match: (Less8U _ (Const8 [0])) + // result: (ConstBool [false]) + for { + if v_1.Op != OpConst8 || auxIntToInt8(v_1.AuxInt) != 0 { + break + } + v.reset(OpConstBool) + v.AuxInt = boolToAuxInt(false) + return true + } return false } func rewriteValuegeneric_OpLoad(v *Value) bool { diff --git a/test/codegen/comparisons.go b/test/codegen/comparisons.go index 90808573c2..f3c15538a8 100644 --- a/test/codegen/comparisons.go +++ b/test/codegen/comparisons.go @@ -407,3 +407,20 @@ func CmpToZero_ex5(e, f int32, u uint32) int { } return 0 } +func UintLtZero(a uint8, b uint16, c uint32, d uint64) int { + // amd64: -`(TESTB|TESTW|TESTL|TESTQ|JCC|JCS)` + // arm64: -`(CMPW|CMP|BHS|BLO)` + if a < 0 || b < 0 || c < 0 || d < 0 { + return 1 + } + return 0 +} + +func UintGeqZero(a uint8, b uint16, c uint32, d uint64) int { + // amd64: -`(TESTB|TESTW|TESTL|TESTQ|JCS|JCC)` + // arm64: -`(CMPW|CMP|BLO|BHS)` + if a >= 0 || b >= 0 || c >= 0 || d >= 0 { + return 1 + } + return 0 +} -- cgit v1.3-5-g9baa From ef9c8a38ad177fa7f48dfaad5d0e27f39a03529d Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Tue, 7 Jul 2020 09:32:12 -0700 Subject: cmd/compile: don't rewrite (CMP (AND x y) 0) to TEST if AND has other uses If the AND has other uses, we end up saving an argument to the AND in another register, so we can use it for the TEST. No point in doing that. Change-Id: I73444a6aeddd6f55e2328ce04d77c3e6cf4a83e0 Reviewed-on: https://go-review.googlesource.com/c/go/+/241280 Run-TryBot: Keith Randall Reviewed-by: Josh Bleecher Snyder TryBot-Result: Gobot Gobot --- src/cmd/compile/internal/ssa/gen/AMD64.rules | 16 ++-- src/cmd/compile/internal/ssa/rewriteAMD64.go | 128 ++++++++++++++++++++------- test/codegen/logic.go | 24 +++++ 3 files changed, 128 insertions(+), 40 deletions(-) create mode 100644 test/codegen/logic.go (limited to 'test') diff --git a/src/cmd/compile/internal/ssa/gen/AMD64.rules b/src/cmd/compile/internal/ssa/gen/AMD64.rules index 9967c7b030..5111ef79d3 100644 --- a/src/cmd/compile/internal/ssa/gen/AMD64.rules +++ b/src/cmd/compile/internal/ssa/gen/AMD64.rules @@ -1463,14 +1463,14 @@ (MULQconst [c] (NEGQ x)) && c != -(1<<31) -> (MULQconst [-c] x) // checking AND against 0. -(CMPQconst (ANDQ x y) [0]) -> (TESTQ x y) -(CMPLconst (ANDL x y) [0]) -> (TESTL x y) -(CMPWconst (ANDL x y) [0]) -> (TESTW x y) -(CMPBconst (ANDL x y) [0]) -> (TESTB x y) -(CMPQconst (ANDQconst [c] x) [0]) -> (TESTQconst [c] x) -(CMPLconst (ANDLconst [c] x) [0]) -> (TESTLconst [c] x) -(CMPWconst (ANDLconst [c] x) [0]) -> (TESTWconst [int64(int16(c))] x) -(CMPBconst (ANDLconst [c] x) [0]) -> (TESTBconst [int64(int8(c))] x) +(CMPQconst a:(ANDQ x y) [0]) && a.Uses == 1 -> (TESTQ x y) +(CMPLconst a:(ANDL x y) [0]) && a.Uses == 1 -> (TESTL x y) +(CMPWconst a:(ANDL x y) [0]) && a.Uses == 1 -> (TESTW x y) +(CMPBconst a:(ANDL x y) [0]) && a.Uses == 1 -> (TESTB x y) +(CMPQconst a:(ANDQconst [c] x) [0]) && a.Uses == 1 -> (TESTQconst [c] x) +(CMPLconst a:(ANDLconst [c] x) [0]) && a.Uses == 1 -> (TESTLconst [c] x) +(CMPWconst a:(ANDLconst [c] x) [0]) && a.Uses == 1 -> (TESTWconst [int64(int16(c))] x) +(CMPBconst a:(ANDLconst [c] x) [0]) && a.Uses == 1 -> (TESTBconst [int64(int8(c))] x) // Convert TESTx to TESTxconst if possible. (TESTQ (MOVQconst [c]) x) && is32Bit(c) -> (TESTQconst [c] x) diff --git a/src/cmd/compile/internal/ssa/rewriteAMD64.go b/src/cmd/compile/internal/ssa/rewriteAMD64.go index 20eab05e9c..cda9df56f4 100644 --- a/src/cmd/compile/internal/ssa/rewriteAMD64.go +++ b/src/cmd/compile/internal/ssa/rewriteAMD64.go @@ -6924,26 +6924,42 @@ func rewriteValueAMD64_OpAMD64CMPBconst(v *Value) bool { v.reset(OpAMD64FlagLT_ULT) return true } - // match: (CMPBconst (ANDL x y) [0]) + // match: (CMPBconst a:(ANDL x y) [0]) + // cond: a.Uses == 1 // result: (TESTB x y) for { - if v.AuxInt != 0 || v_0.Op != OpAMD64ANDL { + if v.AuxInt != 0 { + break + } + a := v_0 + if a.Op != OpAMD64ANDL { + break + } + y := a.Args[1] + x := a.Args[0] + if !(a.Uses == 1) { break } - y := v_0.Args[1] - x := v_0.Args[0] v.reset(OpAMD64TESTB) v.AddArg2(x, y) return true } - // match: (CMPBconst (ANDLconst [c] x) [0]) + // match: (CMPBconst a:(ANDLconst [c] x) [0]) + // cond: a.Uses == 1 // result: (TESTBconst [int64(int8(c))] x) for { - if v.AuxInt != 0 || v_0.Op != OpAMD64ANDLconst { + if v.AuxInt != 0 { + break + } + a := v_0 + if a.Op != OpAMD64ANDLconst { + break + } + c := a.AuxInt + x := a.Args[0] + if !(a.Uses == 1) { break } - c := v_0.AuxInt - x := v_0.Args[0] v.reset(OpAMD64TESTBconst) v.AuxInt = int64(int8(c)) v.AddArg(x) @@ -7309,26 +7325,42 @@ func rewriteValueAMD64_OpAMD64CMPLconst(v *Value) bool { v.reset(OpAMD64FlagLT_ULT) return true } - // match: (CMPLconst (ANDL x y) [0]) + // match: (CMPLconst a:(ANDL x y) [0]) + // cond: a.Uses == 1 // result: (TESTL x y) for { - if v.AuxInt != 0 || v_0.Op != OpAMD64ANDL { + if v.AuxInt != 0 { + break + } + a := v_0 + if a.Op != OpAMD64ANDL { + break + } + y := a.Args[1] + x := a.Args[0] + if !(a.Uses == 1) { break } - y := v_0.Args[1] - x := v_0.Args[0] v.reset(OpAMD64TESTL) v.AddArg2(x, y) return true } - // match: (CMPLconst (ANDLconst [c] x) [0]) + // match: (CMPLconst a:(ANDLconst [c] x) [0]) + // cond: a.Uses == 1 // result: (TESTLconst [c] x) for { - if v.AuxInt != 0 || v_0.Op != OpAMD64ANDLconst { + if v.AuxInt != 0 { + break + } + a := v_0 + if a.Op != OpAMD64ANDLconst { + break + } + c := a.AuxInt + x := a.Args[0] + if !(a.Uses == 1) { break } - c := v_0.AuxInt - x := v_0.Args[0] v.reset(OpAMD64TESTLconst) v.AuxInt = c v.AddArg(x) @@ -7874,26 +7906,42 @@ func rewriteValueAMD64_OpAMD64CMPQconst(v *Value) bool { v.reset(OpAMD64FlagLT_ULT) return true } - // match: (CMPQconst (ANDQ x y) [0]) + // match: (CMPQconst a:(ANDQ x y) [0]) + // cond: a.Uses == 1 // result: (TESTQ x y) for { - if v.AuxInt != 0 || v_0.Op != OpAMD64ANDQ { + if v.AuxInt != 0 { + break + } + a := v_0 + if a.Op != OpAMD64ANDQ { + break + } + y := a.Args[1] + x := a.Args[0] + if !(a.Uses == 1) { break } - y := v_0.Args[1] - x := v_0.Args[0] v.reset(OpAMD64TESTQ) v.AddArg2(x, y) return true } - // match: (CMPQconst (ANDQconst [c] x) [0]) + // match: (CMPQconst a:(ANDQconst [c] x) [0]) + // cond: a.Uses == 1 // result: (TESTQconst [c] x) for { - if v.AuxInt != 0 || v_0.Op != OpAMD64ANDQconst { + if v.AuxInt != 0 { + break + } + a := v_0 + if a.Op != OpAMD64ANDQconst { + break + } + c := a.AuxInt + x := a.Args[0] + if !(a.Uses == 1) { break } - c := v_0.AuxInt - x := v_0.Args[0] v.reset(OpAMD64TESTQconst) v.AuxInt = c v.AddArg(x) @@ -8244,26 +8292,42 @@ func rewriteValueAMD64_OpAMD64CMPWconst(v *Value) bool { v.reset(OpAMD64FlagLT_ULT) return true } - // match: (CMPWconst (ANDL x y) [0]) + // match: (CMPWconst a:(ANDL x y) [0]) + // cond: a.Uses == 1 // result: (TESTW x y) for { - if v.AuxInt != 0 || v_0.Op != OpAMD64ANDL { + if v.AuxInt != 0 { + break + } + a := v_0 + if a.Op != OpAMD64ANDL { + break + } + y := a.Args[1] + x := a.Args[0] + if !(a.Uses == 1) { break } - y := v_0.Args[1] - x := v_0.Args[0] v.reset(OpAMD64TESTW) v.AddArg2(x, y) return true } - // match: (CMPWconst (ANDLconst [c] x) [0]) + // match: (CMPWconst a:(ANDLconst [c] x) [0]) + // cond: a.Uses == 1 // result: (TESTWconst [int64(int16(c))] x) for { - if v.AuxInt != 0 || v_0.Op != OpAMD64ANDLconst { + if v.AuxInt != 0 { + break + } + a := v_0 + if a.Op != OpAMD64ANDLconst { + break + } + c := a.AuxInt + x := a.Args[0] + if !(a.Uses == 1) { break } - c := v_0.AuxInt - x := v_0.Args[0] v.reset(OpAMD64TESTWconst) v.AuxInt = int64(int16(c)) v.AddArg(x) diff --git a/test/codegen/logic.go b/test/codegen/logic.go new file mode 100644 index 0000000000..9afdfd760f --- /dev/null +++ b/test/codegen/logic.go @@ -0,0 +1,24 @@ +// asmcheck + +// Copyright 2018 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 codegen + +var gx, gy int + +// Test to make sure that (CMPQ (ANDQ x y) [0]) does not get rewritten to +// (TESTQ x y) if the ANDQ has other uses. If that rewrite happens, then one +// of the args of the ANDQ needs to be saved so it can be used as the arg to TESTQ. +func andWithUse(x, y int) int { + // Load x,y into registers, so those MOVQ will not appear at the z := x&y line. + gx, gy = x, y + // amd64:-"MOVQ" + z := x & y + if z == 0 { + return 77 + } + // use z by returning it + return z +} -- cgit v1.3-5-g9baa From 06337823ef4d9c57b1b01f9b97a348a47277a9df Mon Sep 17 00:00:00 2001 From: Junchen Li Date: Fri, 10 Jul 2020 11:39:23 +0800 Subject: cmd/compile: optimize unsigned comparisons to 0/1 on arm64 For an unsigned integer, it's useful to convert its order test with 0/1 to its equality test with 0. We can save a comparison instruction that followed by a conditional branch on arm64 since it supports compare-with-zero-and-branch instructions. For example, if x > 0 { ... } else { ... } the original version: CMP $0, R0 BLS 9 the optimized version: CBZ R0, 8 Updates #21439 Change-Id: Id1de6f865f6aa72c5d45b29f7894818857288425 Reviewed-on: https://go-review.googlesource.com/c/go/+/246857 Reviewed-by: Keith Randall --- src/cmd/compile/internal/ssa/gen/ARM64.rules | 10 ++ src/cmd/compile/internal/ssa/rewriteARM64.go | 204 +++++++++++++++++++++++++++ test/codegen/comparisons.go | 32 +++++ 3 files changed, 246 insertions(+) (limited to 'test') diff --git a/src/cmd/compile/internal/ssa/gen/ARM64.rules b/src/cmd/compile/internal/ssa/gen/ARM64.rules index 442d769fdd..27959d01fc 100644 --- a/src/cmd/compile/internal/ssa/gen/ARM64.rules +++ b/src/cmd/compile/internal/ssa/gen/ARM64.rules @@ -279,6 +279,16 @@ (Less32F x y) => (LessThanF (FCMPS x y)) (Less64F x y) => (LessThanF (FCMPD x y)) +// For an unsigned integer x, the following rules are useful when combining branch +// 0 < x => x != 0 +// x <= 0 => x == 0 +// x < 1 => x == 0 +// 1 <= x => x != 0 +(Less(8U|16U|32U|64U) zero:(MOVDconst [0]) x) => (Neq(8|16|32|64) zero x) +(Leq(8U|16U|32U|64U) x zero:(MOVDconst [0])) => (Eq(8|16|32|64) x zero) +(Less(8U|16U|32U|64U) x (MOVDconst [1])) => (Eq(8|16|32|64) x (MOVDconst [0])) +(Leq(8U|16U|32U|64U) (MOVDconst [1]) x) => (Neq(8|16|32|64) (MOVDconst [0]) x) + (Less8U x y) => (LessThanU (CMPW (ZeroExt8to32 x) (ZeroExt8to32 y))) (Less16U x y) => (LessThanU (CMPW (ZeroExt16to32 x) (ZeroExt16to32 y))) (Less32U x y) => (LessThanU (CMPW x y)) diff --git a/src/cmd/compile/internal/ssa/rewriteARM64.go b/src/cmd/compile/internal/ssa/rewriteARM64.go index 8e48b33628..023d9908c2 100644 --- a/src/cmd/compile/internal/ssa/rewriteARM64.go +++ b/src/cmd/compile/internal/ssa/rewriteARM64.go @@ -21976,6 +21976,31 @@ func rewriteValueARM64_OpLeq16U(v *Value) bool { v_0 := v.Args[0] b := v.Block typ := &b.Func.Config.Types + // match: (Leq16U x zero:(MOVDconst [0])) + // result: (Eq16 x zero) + for { + x := v_0 + zero := v_1 + if zero.Op != OpARM64MOVDconst || auxIntToInt64(zero.AuxInt) != 0 { + break + } + v.reset(OpEq16) + v.AddArg2(x, zero) + return true + } + // match: (Leq16U (MOVDconst [1]) x) + // result: (Neq16 (MOVDconst [0]) x) + for { + if v_0.Op != OpARM64MOVDconst || auxIntToInt64(v_0.AuxInt) != 1 { + break + } + x := v_1 + v.reset(OpNeq16) + v0 := b.NewValue0(v.Pos, OpARM64MOVDconst, typ.UInt64) + v0.AuxInt = int64ToAuxInt(0) + v.AddArg2(v0, x) + return true + } // match: (Leq16U x y) // result: (LessEqualU (CMPW (ZeroExt16to32 x) (ZeroExt16to32 y))) for { @@ -22028,6 +22053,32 @@ func rewriteValueARM64_OpLeq32U(v *Value) bool { v_1 := v.Args[1] v_0 := v.Args[0] b := v.Block + typ := &b.Func.Config.Types + // match: (Leq32U x zero:(MOVDconst [0])) + // result: (Eq32 x zero) + for { + x := v_0 + zero := v_1 + if zero.Op != OpARM64MOVDconst || auxIntToInt64(zero.AuxInt) != 0 { + break + } + v.reset(OpEq32) + v.AddArg2(x, zero) + return true + } + // match: (Leq32U (MOVDconst [1]) x) + // result: (Neq32 (MOVDconst [0]) x) + for { + if v_0.Op != OpARM64MOVDconst || auxIntToInt64(v_0.AuxInt) != 1 { + break + } + x := v_1 + v.reset(OpNeq32) + v0 := b.NewValue0(v.Pos, OpARM64MOVDconst, typ.UInt64) + v0.AuxInt = int64ToAuxInt(0) + v.AddArg2(v0, x) + return true + } // match: (Leq32U x y) // result: (LessEqualU (CMPW x y)) for { @@ -22076,6 +22127,32 @@ func rewriteValueARM64_OpLeq64U(v *Value) bool { v_1 := v.Args[1] v_0 := v.Args[0] b := v.Block + typ := &b.Func.Config.Types + // match: (Leq64U x zero:(MOVDconst [0])) + // result: (Eq64 x zero) + for { + x := v_0 + zero := v_1 + if zero.Op != OpARM64MOVDconst || auxIntToInt64(zero.AuxInt) != 0 { + break + } + v.reset(OpEq64) + v.AddArg2(x, zero) + return true + } + // match: (Leq64U (MOVDconst [1]) x) + // result: (Neq64 (MOVDconst [0]) x) + for { + if v_0.Op != OpARM64MOVDconst || auxIntToInt64(v_0.AuxInt) != 1 { + break + } + x := v_1 + v.reset(OpNeq64) + v0 := b.NewValue0(v.Pos, OpARM64MOVDconst, typ.UInt64) + v0.AuxInt = int64ToAuxInt(0) + v.AddArg2(v0, x) + return true + } // match: (Leq64U x y) // result: (LessEqualU (CMP x y)) for { @@ -22114,6 +22191,31 @@ func rewriteValueARM64_OpLeq8U(v *Value) bool { v_0 := v.Args[0] b := v.Block typ := &b.Func.Config.Types + // match: (Leq8U x zero:(MOVDconst [0])) + // result: (Eq8 x zero) + for { + x := v_0 + zero := v_1 + if zero.Op != OpARM64MOVDconst || auxIntToInt64(zero.AuxInt) != 0 { + break + } + v.reset(OpEq8) + v.AddArg2(x, zero) + return true + } + // match: (Leq8U (MOVDconst [1]) x) + // result: (Neq8 (MOVDconst [0]) x) + for { + if v_0.Op != OpARM64MOVDconst || auxIntToInt64(v_0.AuxInt) != 1 { + break + } + x := v_1 + v.reset(OpNeq8) + v0 := b.NewValue0(v.Pos, OpARM64MOVDconst, typ.UInt64) + v0.AuxInt = int64ToAuxInt(0) + v.AddArg2(v0, x) + return true + } // match: (Leq8U x y) // result: (LessEqualU (CMPW (ZeroExt8to32 x) (ZeroExt8to32 y))) for { @@ -22156,6 +22258,31 @@ func rewriteValueARM64_OpLess16U(v *Value) bool { v_0 := v.Args[0] b := v.Block typ := &b.Func.Config.Types + // match: (Less16U zero:(MOVDconst [0]) x) + // result: (Neq16 zero x) + for { + zero := v_0 + if zero.Op != OpARM64MOVDconst || auxIntToInt64(zero.AuxInt) != 0 { + break + } + x := v_1 + v.reset(OpNeq16) + v.AddArg2(zero, x) + return true + } + // match: (Less16U x (MOVDconst [1])) + // result: (Eq16 x (MOVDconst [0])) + for { + x := v_0 + if v_1.Op != OpARM64MOVDconst || auxIntToInt64(v_1.AuxInt) != 1 { + break + } + v.reset(OpEq16) + v0 := b.NewValue0(v.Pos, OpARM64MOVDconst, typ.UInt64) + v0.AuxInt = int64ToAuxInt(0) + v.AddArg2(x, v0) + return true + } // match: (Less16U x y) // result: (LessThanU (CMPW (ZeroExt16to32 x) (ZeroExt16to32 y))) for { @@ -22208,6 +22335,32 @@ func rewriteValueARM64_OpLess32U(v *Value) bool { v_1 := v.Args[1] v_0 := v.Args[0] b := v.Block + typ := &b.Func.Config.Types + // match: (Less32U zero:(MOVDconst [0]) x) + // result: (Neq32 zero x) + for { + zero := v_0 + if zero.Op != OpARM64MOVDconst || auxIntToInt64(zero.AuxInt) != 0 { + break + } + x := v_1 + v.reset(OpNeq32) + v.AddArg2(zero, x) + return true + } + // match: (Less32U x (MOVDconst [1])) + // result: (Eq32 x (MOVDconst [0])) + for { + x := v_0 + if v_1.Op != OpARM64MOVDconst || auxIntToInt64(v_1.AuxInt) != 1 { + break + } + v.reset(OpEq32) + v0 := b.NewValue0(v.Pos, OpARM64MOVDconst, typ.UInt64) + v0.AuxInt = int64ToAuxInt(0) + v.AddArg2(x, v0) + return true + } // match: (Less32U x y) // result: (LessThanU (CMPW x y)) for { @@ -22256,6 +22409,32 @@ func rewriteValueARM64_OpLess64U(v *Value) bool { v_1 := v.Args[1] v_0 := v.Args[0] b := v.Block + typ := &b.Func.Config.Types + // match: (Less64U zero:(MOVDconst [0]) x) + // result: (Neq64 zero x) + for { + zero := v_0 + if zero.Op != OpARM64MOVDconst || auxIntToInt64(zero.AuxInt) != 0 { + break + } + x := v_1 + v.reset(OpNeq64) + v.AddArg2(zero, x) + return true + } + // match: (Less64U x (MOVDconst [1])) + // result: (Eq64 x (MOVDconst [0])) + for { + x := v_0 + if v_1.Op != OpARM64MOVDconst || auxIntToInt64(v_1.AuxInt) != 1 { + break + } + v.reset(OpEq64) + v0 := b.NewValue0(v.Pos, OpARM64MOVDconst, typ.UInt64) + v0.AuxInt = int64ToAuxInt(0) + v.AddArg2(x, v0) + return true + } // match: (Less64U x y) // result: (LessThanU (CMP x y)) for { @@ -22294,6 +22473,31 @@ func rewriteValueARM64_OpLess8U(v *Value) bool { v_0 := v.Args[0] b := v.Block typ := &b.Func.Config.Types + // match: (Less8U zero:(MOVDconst [0]) x) + // result: (Neq8 zero x) + for { + zero := v_0 + if zero.Op != OpARM64MOVDconst || auxIntToInt64(zero.AuxInt) != 0 { + break + } + x := v_1 + v.reset(OpNeq8) + v.AddArg2(zero, x) + return true + } + // match: (Less8U x (MOVDconst [1])) + // result: (Eq8 x (MOVDconst [0])) + for { + x := v_0 + if v_1.Op != OpARM64MOVDconst || auxIntToInt64(v_1.AuxInt) != 1 { + break + } + v.reset(OpEq8) + v0 := b.NewValue0(v.Pos, OpARM64MOVDconst, typ.UInt64) + v0.AuxInt = int64ToAuxInt(0) + v.AddArg2(x, v0) + return true + } // match: (Less8U x y) // result: (LessThanU (CMPW (ZeroExt8to32 x) (ZeroExt8to32 y))) for { diff --git a/test/codegen/comparisons.go b/test/codegen/comparisons.go index f3c15538a8..3c2dcb7eba 100644 --- a/test/codegen/comparisons.go +++ b/test/codegen/comparisons.go @@ -424,3 +424,35 @@ func UintGeqZero(a uint8, b uint16, c uint32, d uint64) int { } return 0 } + +func UintGtZero(a uint8, b uint16, c uint32, d uint64) int { + // arm64: `CBZW`, `CBNZW`, `CBNZ`, -`(CMPW|CMP|BLS|BHI)` + if a > 0 || b > 0 || c > 0 || d > 0 { + return 1 + } + return 0 +} + +func UintLeqZero(a uint8, b uint16, c uint32, d uint64) int { + // arm64: `CBNZW`, `CBZW`, `CBZ`, -`(CMPW|CMP|BHI|BLS)` + if a <= 0 || b <= 0 || c <= 0 || d <= 0 { + return 1 + } + return 0 +} + +func UintLtOne(a uint8, b uint16, c uint32, d uint64) int { + // arm64: `CBNZW`, `CBZW`, `CBZW`, `CBZ`, -`(CMPW|CMP|BHS|BLO)` + if a < 1 || b < 1 || c < 1 || d < 1 { + return 1 + } + return 0 +} + +func UintGeqOne(a uint8, b uint16, c uint32, d uint64) int { + // arm64: `CBZW`, `CBNZW`, `CBNZ`, -`(CMPW|CMP|BLO|BHS)` + if a >= 1 || b >= 1 || c >= 1 || d >= 1 { + return 1 + } + return 0 +} -- cgit v1.3-5-g9baa From 6e876f19857a8fbd259571080f7f91bc03276559 Mon Sep 17 00:00:00 2001 From: Michael Munday Date: Thu, 4 Jun 2020 10:55:01 -0700 Subject: cmd/compile: clean up and optimize s390x multiplication rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some of the existing optimizations aren't triggered because they are handled by the generic rules so this CL removes them. Also some constraints were copied without much thought from the amd64 rules and they don't make sense on s390x, so we remove those constraints. Finally, add a 'multiply by the sum of two powers of two' optimization. This makes sense on s390x as shifts are low latency and can also sometimes be optimized further (especially if we add support for RISBG instructions). name old time/op new time/op delta IntMulByConst/3-8 1.70ns ±11% 1.10ns ± 5% -35.26% (p=0.000 n=10+10) IntMulByConst/5-8 1.64ns ± 7% 1.10ns ± 4% -32.94% (p=0.000 n=10+9) IntMulByConst/12-8 1.65ns ± 6% 1.20ns ± 4% -27.16% (p=0.000 n=10+9) IntMulByConst/120-8 1.66ns ± 4% 1.22ns ±13% -26.43% (p=0.000 n=10+10) IntMulByConst/-120-8 1.65ns ± 7% 1.19ns ± 4% -28.06% (p=0.000 n=9+10) IntMulByConst/65537-8 0.86ns ± 9% 1.12ns ±12% +30.41% (p=0.000 n=10+10) IntMulByConst/65538-8 1.65ns ± 5% 1.23ns ± 5% -25.11% (p=0.000 n=10+10) Change-Id: Ib196e6bff1e97febfd266134d0a2b2a62897989f Reviewed-on: https://go-review.googlesource.com/c/go/+/248937 Run-TryBot: Michael Munday TryBot-Result: Gobot Gobot Reviewed-by: Keith Randall --- src/cmd/compile/internal/ssa/gen/S390X.rules | 51 +++-- src/cmd/compile/internal/ssa/rewriteS390X.go | 268 ++++++++++++++----------- src/cmd/compile/internal/test/mulconst_test.go | 242 ++++++++++++++++++++++ test/codegen/arithmetic.go | 6 + 4 files changed, 441 insertions(+), 126 deletions(-) create mode 100644 src/cmd/compile/internal/test/mulconst_test.go (limited to 'test') diff --git a/src/cmd/compile/internal/ssa/gen/S390X.rules b/src/cmd/compile/internal/ssa/gen/S390X.rules index d3234c1a00..5e4c436ca1 100644 --- a/src/cmd/compile/internal/ssa/gen/S390X.rules +++ b/src/cmd/compile/internal/ssa/gen/S390X.rules @@ -716,20 +716,40 @@ (ANDWconst [0xFF] x) => (MOVBZreg x) (ANDWconst [0xFFFF] x) => (MOVHZreg x) -// strength reduction -(MULLDconst [-1] x) => (NEG x) -(MULLDconst [0] _) => (MOVDconst [0]) -(MULLDconst [1] x) => x -(MULLDconst [c] x) && isPowerOfTwo(c) -> (SLDconst [log2(c)] x) -(MULLDconst [c] x) && isPowerOfTwo(c+1) && c >= 15 -> (SUB (SLDconst [log2(c+1)] x) x) -(MULLDconst [c] x) && isPowerOfTwo(c-1) && c >= 17 -> (ADD (SLDconst [log2(c-1)] x) x) - -(MULLWconst [-1] x) => (NEGW x) -(MULLWconst [0] _) => (MOVDconst [0]) -(MULLWconst [1] x) => x -(MULLWconst [c] x) && isPowerOfTwo(c) -> (SLWconst [log2(c)] x) -(MULLWconst [c] x) && isPowerOfTwo(c+1) && c >= 15 -> (SUBW (SLWconst [log2(c+1)] x) x) -(MULLWconst [c] x) && isPowerOfTwo(c-1) && c >= 17 -> (ADDW (SLWconst [log2(c-1)] x) x) +// Strength reduce multiplication to the sum (or difference) of two powers of two. +// +// Examples: +// 5x -> 4x + 1x +// 10x -> 8x + 2x +// 120x -> 128x - 8x +// -120x -> 8x - 128x +// +// We know that the rightmost bit of any positive value, once isolated, must either +// be a power of 2 (because it is a single bit) or 0 (if the original value is 0). +// In all of these rules we use a rightmost bit calculation to determine one operand +// for the addition or subtraction. We then just need to calculate if the other +// operand is a valid power of 2 before we can match the rule. +// +// Notes: +// - the generic rules have already matched single powers of two so we ignore them here +// - isPowerOfTwo32 asserts that its argument is greater than 0 +// - c&(c-1) = clear rightmost bit +// - c&^(c-1) = isolate rightmost bit + +// c = 2ˣ + 2ʸ => c - 2ˣ = 2ʸ +(MULL(D|W)const x [c]) && isPowerOfTwo32(c&(c-1)) + => ((ADD|ADDW) (SL(D|W)const x [int8(log32(c&(c-1)))]) + (SL(D|W)const x [int8(log32(c&^(c-1)))])) + +// c = 2ʸ - 2ˣ => c + 2ˣ = 2ʸ +(MULL(D|W)const x [c]) && isPowerOfTwo32(c+(c&^(c-1))) + => ((SUB|SUBW) (SL(D|W)const x [int8(log32(c+(c&^(c-1))))]) + (SL(D|W)const x [int8(log32(c&^(c-1)))])) + +// c = 2ˣ - 2ʸ => -c + 2ˣ = 2ʸ +(MULL(D|W)const x [c]) && isPowerOfTwo32(-c+(-c&^(-c-1))) + => ((SUB|SUBW) (SL(D|W)const x [int8(log32(-c&^(-c-1)))]) + (SL(D|W)const x [int8(log32(-c+(-c&^(-c-1))))])) // Fold ADD into MOVDaddr. Odd offsets from SB shouldn't be folded (LARL can't handle them). (ADDconst [c] (MOVDaddr [d] {s} x:(SB))) && ((c+d)&1 == 0) && is32Bit(c+d) -> (MOVDaddr [c+d] {s} x) @@ -1133,6 +1153,9 @@ (XORconst [0] x) => x (XORWconst [c] x) && int32(c)==0 => x +// Shifts by zero (may be inserted during multiplication strength reduction). +((SLD|SLW|SRD|SRW|SRAD|SRAW)const x [0]) => x + // Convert constant subtracts to constant adds. (SUBconst [c] x) && c != -(1<<31) => (ADDconst [-c] x) (SUBWconst [c] x) -> (ADDWconst [int64(int32(-c))] x) diff --git a/src/cmd/compile/internal/ssa/rewriteS390X.go b/src/cmd/compile/internal/ssa/rewriteS390X.go index dc9b143562..536f8db320 100644 --- a/src/cmd/compile/internal/ssa/rewriteS390X.go +++ b/src/cmd/compile/internal/ssa/rewriteS390X.go @@ -732,8 +732,12 @@ func rewriteValueS390X(v *Value) bool { return rewriteValueS390X_OpS390XRLLG(v) case OpS390XSLD: return rewriteValueS390X_OpS390XSLD(v) + case OpS390XSLDconst: + return rewriteValueS390X_OpS390XSLDconst(v) case OpS390XSLW: return rewriteValueS390X_OpS390XSLW(v) + case OpS390XSLWconst: + return rewriteValueS390X_OpS390XSLWconst(v) case OpS390XSRAD: return rewriteValueS390X_OpS390XSRAD(v) case OpS390XSRADconst: @@ -748,6 +752,8 @@ func rewriteValueS390X(v *Value) bool { return rewriteValueS390X_OpS390XSRDconst(v) case OpS390XSRW: return rewriteValueS390X_OpS390XSRW(v) + case OpS390XSRWconst: + return rewriteValueS390X_OpS390XSRWconst(v) case OpS390XSTM2: return rewriteValueS390X_OpS390XSTM2(v) case OpS390XSTMG2: @@ -13853,81 +13859,64 @@ func rewriteValueS390X_OpS390XMULLD(v *Value) bool { func rewriteValueS390X_OpS390XMULLDconst(v *Value) bool { v_0 := v.Args[0] b := v.Block - // match: (MULLDconst [-1] x) - // result: (NEG x) + // match: (MULLDconst x [c]) + // cond: isPowerOfTwo32(c&(c-1)) + // result: (ADD (SLDconst x [int8(log32(c&(c-1)))]) (SLDconst x [int8(log32(c&^(c-1)))])) for { - if auxIntToInt32(v.AuxInt) != -1 { - break - } - x := v_0 - v.reset(OpS390XNEG) - v.AddArg(x) - return true - } - // match: (MULLDconst [0] _) - // result: (MOVDconst [0]) - for { - if auxIntToInt32(v.AuxInt) != 0 { - break - } - v.reset(OpS390XMOVDconst) - v.AuxInt = int64ToAuxInt(0) - return true - } - // match: (MULLDconst [1] x) - // result: x - for { - if auxIntToInt32(v.AuxInt) != 1 { - break - } - x := v_0 - v.copyOf(x) - return true - } - // match: (MULLDconst [c] x) - // cond: isPowerOfTwo(c) - // result: (SLDconst [log2(c)] x) - for { - c := v.AuxInt + t := v.Type + c := auxIntToInt32(v.AuxInt) x := v_0 - if !(isPowerOfTwo(c)) { + if !(isPowerOfTwo32(c & (c - 1))) { break } - v.reset(OpS390XSLDconst) - v.AuxInt = log2(c) - v.AddArg(x) + v.reset(OpS390XADD) + v0 := b.NewValue0(v.Pos, OpS390XSLDconst, t) + v0.AuxInt = int8ToAuxInt(int8(log32(c & (c - 1)))) + v0.AddArg(x) + v1 := b.NewValue0(v.Pos, OpS390XSLDconst, t) + v1.AuxInt = int8ToAuxInt(int8(log32(c &^ (c - 1)))) + v1.AddArg(x) + v.AddArg2(v0, v1) return true } - // match: (MULLDconst [c] x) - // cond: isPowerOfTwo(c+1) && c >= 15 - // result: (SUB (SLDconst [log2(c+1)] x) x) + // match: (MULLDconst x [c]) + // cond: isPowerOfTwo32(c+(c&^(c-1))) + // result: (SUB (SLDconst x [int8(log32(c+(c&^(c-1))))]) (SLDconst x [int8(log32(c&^(c-1)))])) for { - c := v.AuxInt + t := v.Type + c := auxIntToInt32(v.AuxInt) x := v_0 - if !(isPowerOfTwo(c+1) && c >= 15) { + if !(isPowerOfTwo32(c + (c &^ (c - 1)))) { break } v.reset(OpS390XSUB) - v0 := b.NewValue0(v.Pos, OpS390XSLDconst, v.Type) - v0.AuxInt = log2(c + 1) + v0 := b.NewValue0(v.Pos, OpS390XSLDconst, t) + v0.AuxInt = int8ToAuxInt(int8(log32(c + (c &^ (c - 1))))) v0.AddArg(x) - v.AddArg2(v0, x) + v1 := b.NewValue0(v.Pos, OpS390XSLDconst, t) + v1.AuxInt = int8ToAuxInt(int8(log32(c &^ (c - 1)))) + v1.AddArg(x) + v.AddArg2(v0, v1) return true } - // match: (MULLDconst [c] x) - // cond: isPowerOfTwo(c-1) && c >= 17 - // result: (ADD (SLDconst [log2(c-1)] x) x) + // match: (MULLDconst x [c]) + // cond: isPowerOfTwo32(-c+(-c&^(-c-1))) + // result: (SUB (SLDconst x [int8(log32(-c&^(-c-1)))]) (SLDconst x [int8(log32(-c+(-c&^(-c-1))))])) for { - c := v.AuxInt + t := v.Type + c := auxIntToInt32(v.AuxInt) x := v_0 - if !(isPowerOfTwo(c-1) && c >= 17) { + if !(isPowerOfTwo32(-c + (-c &^ (-c - 1)))) { break } - v.reset(OpS390XADD) - v0 := b.NewValue0(v.Pos, OpS390XSLDconst, v.Type) - v0.AuxInt = log2(c - 1) + v.reset(OpS390XSUB) + v0 := b.NewValue0(v.Pos, OpS390XSLDconst, t) + v0.AuxInt = int8ToAuxInt(int8(log32(-c &^ (-c - 1)))) v0.AddArg(x) - v.AddArg2(v0, x) + v1 := b.NewValue0(v.Pos, OpS390XSLDconst, t) + v1.AuxInt = int8ToAuxInt(int8(log32(-c + (-c &^ (-c - 1))))) + v1.AddArg(x) + v.AddArg2(v0, v1) return true } // match: (MULLDconst [c] (MOVDconst [d])) @@ -14097,81 +14086,64 @@ func rewriteValueS390X_OpS390XMULLW(v *Value) bool { func rewriteValueS390X_OpS390XMULLWconst(v *Value) bool { v_0 := v.Args[0] b := v.Block - // match: (MULLWconst [-1] x) - // result: (NEGW x) - for { - if auxIntToInt32(v.AuxInt) != -1 { - break - } - x := v_0 - v.reset(OpS390XNEGW) - v.AddArg(x) - return true - } - // match: (MULLWconst [0] _) - // result: (MOVDconst [0]) - for { - if auxIntToInt32(v.AuxInt) != 0 { - break - } - v.reset(OpS390XMOVDconst) - v.AuxInt = int64ToAuxInt(0) - return true - } - // match: (MULLWconst [1] x) - // result: x + // match: (MULLWconst x [c]) + // cond: isPowerOfTwo32(c&(c-1)) + // result: (ADDW (SLWconst x [int8(log32(c&(c-1)))]) (SLWconst x [int8(log32(c&^(c-1)))])) for { - if auxIntToInt32(v.AuxInt) != 1 { - break - } - x := v_0 - v.copyOf(x) - return true - } - // match: (MULLWconst [c] x) - // cond: isPowerOfTwo(c) - // result: (SLWconst [log2(c)] x) - for { - c := v.AuxInt + t := v.Type + c := auxIntToInt32(v.AuxInt) x := v_0 - if !(isPowerOfTwo(c)) { + if !(isPowerOfTwo32(c & (c - 1))) { break } - v.reset(OpS390XSLWconst) - v.AuxInt = log2(c) - v.AddArg(x) + v.reset(OpS390XADDW) + v0 := b.NewValue0(v.Pos, OpS390XSLWconst, t) + v0.AuxInt = int8ToAuxInt(int8(log32(c & (c - 1)))) + v0.AddArg(x) + v1 := b.NewValue0(v.Pos, OpS390XSLWconst, t) + v1.AuxInt = int8ToAuxInt(int8(log32(c &^ (c - 1)))) + v1.AddArg(x) + v.AddArg2(v0, v1) return true } - // match: (MULLWconst [c] x) - // cond: isPowerOfTwo(c+1) && c >= 15 - // result: (SUBW (SLWconst [log2(c+1)] x) x) + // match: (MULLWconst x [c]) + // cond: isPowerOfTwo32(c+(c&^(c-1))) + // result: (SUBW (SLWconst x [int8(log32(c+(c&^(c-1))))]) (SLWconst x [int8(log32(c&^(c-1)))])) for { - c := v.AuxInt + t := v.Type + c := auxIntToInt32(v.AuxInt) x := v_0 - if !(isPowerOfTwo(c+1) && c >= 15) { + if !(isPowerOfTwo32(c + (c &^ (c - 1)))) { break } v.reset(OpS390XSUBW) - v0 := b.NewValue0(v.Pos, OpS390XSLWconst, v.Type) - v0.AuxInt = log2(c + 1) + v0 := b.NewValue0(v.Pos, OpS390XSLWconst, t) + v0.AuxInt = int8ToAuxInt(int8(log32(c + (c &^ (c - 1))))) v0.AddArg(x) - v.AddArg2(v0, x) + v1 := b.NewValue0(v.Pos, OpS390XSLWconst, t) + v1.AuxInt = int8ToAuxInt(int8(log32(c &^ (c - 1)))) + v1.AddArg(x) + v.AddArg2(v0, v1) return true } - // match: (MULLWconst [c] x) - // cond: isPowerOfTwo(c-1) && c >= 17 - // result: (ADDW (SLWconst [log2(c-1)] x) x) + // match: (MULLWconst x [c]) + // cond: isPowerOfTwo32(-c+(-c&^(-c-1))) + // result: (SUBW (SLWconst x [int8(log32(-c&^(-c-1)))]) (SLWconst x [int8(log32(-c+(-c&^(-c-1))))])) for { - c := v.AuxInt + t := v.Type + c := auxIntToInt32(v.AuxInt) x := v_0 - if !(isPowerOfTwo(c-1) && c >= 17) { + if !(isPowerOfTwo32(-c + (-c &^ (-c - 1)))) { break } - v.reset(OpS390XADDW) - v0 := b.NewValue0(v.Pos, OpS390XSLWconst, v.Type) - v0.AuxInt = log2(c - 1) + v.reset(OpS390XSUBW) + v0 := b.NewValue0(v.Pos, OpS390XSLWconst, t) + v0.AuxInt = int8ToAuxInt(int8(log32(-c &^ (-c - 1)))) v0.AddArg(x) - v.AddArg2(v0, x) + v1 := b.NewValue0(v.Pos, OpS390XSLWconst, t) + v1.AuxInt = int8ToAuxInt(int8(log32(-c + (-c &^ (-c - 1))))) + v1.AddArg(x) + v.AddArg2(v0, v1) return true } // match: (MULLWconst [c] (MOVDconst [d])) @@ -16826,6 +16798,20 @@ func rewriteValueS390X_OpS390XSLD(v *Value) bool { } return false } +func rewriteValueS390X_OpS390XSLDconst(v *Value) bool { + v_0 := v.Args[0] + // match: (SLDconst x [0]) + // result: x + for { + if auxIntToInt8(v.AuxInt) != 0 { + break + } + x := v_0 + v.copyOf(x) + return true + } + return false +} func rewriteValueS390X_OpS390XSLW(v *Value) bool { v_1 := v.Args[1] v_0 := v.Args[0] @@ -16960,6 +16946,20 @@ func rewriteValueS390X_OpS390XSLW(v *Value) bool { } return false } +func rewriteValueS390X_OpS390XSLWconst(v *Value) bool { + v_0 := v.Args[0] + // match: (SLWconst x [0]) + // result: x + for { + if auxIntToInt8(v.AuxInt) != 0 { + break + } + x := v_0 + v.copyOf(x) + return true + } + return false +} func rewriteValueS390X_OpS390XSRAD(v *Value) bool { v_1 := v.Args[1] v_0 := v.Args[0] @@ -17096,6 +17096,16 @@ func rewriteValueS390X_OpS390XSRAD(v *Value) bool { } func rewriteValueS390X_OpS390XSRADconst(v *Value) bool { v_0 := v.Args[0] + // match: (SRADconst x [0]) + // result: x + for { + if auxIntToInt8(v.AuxInt) != 0 { + break + } + x := v_0 + v.copyOf(x) + return true + } // match: (SRADconst [c] (MOVDconst [d])) // result: (MOVDconst [d>>uint64(c)]) for { @@ -17246,6 +17256,16 @@ func rewriteValueS390X_OpS390XSRAW(v *Value) bool { } func rewriteValueS390X_OpS390XSRAWconst(v *Value) bool { v_0 := v.Args[0] + // match: (SRAWconst x [0]) + // result: x + for { + if auxIntToInt8(v.AuxInt) != 0 { + break + } + x := v_0 + v.copyOf(x) + return true + } // match: (SRAWconst [c] (MOVDconst [d])) // result: (MOVDconst [int64(int32(d))>>uint64(c)]) for { @@ -17416,6 +17436,16 @@ func rewriteValueS390X_OpS390XSRDconst(v *Value) bool { v.AddArg(v0) return true } + // match: (SRDconst x [0]) + // result: x + for { + if auxIntToInt8(v.AuxInt) != 0 { + break + } + x := v_0 + v.copyOf(x) + return true + } return false } func rewriteValueS390X_OpS390XSRW(v *Value) bool { @@ -17552,6 +17582,20 @@ func rewriteValueS390X_OpS390XSRW(v *Value) bool { } return false } +func rewriteValueS390X_OpS390XSRWconst(v *Value) bool { + v_0 := v.Args[0] + // match: (SRWconst x [0]) + // result: x + for { + if auxIntToInt8(v.AuxInt) != 0 { + break + } + x := v_0 + v.copyOf(x) + return true + } + return false +} func rewriteValueS390X_OpS390XSTM2(v *Value) bool { v_3 := v.Args[3] v_2 := v.Args[2] diff --git a/src/cmd/compile/internal/test/mulconst_test.go b/src/cmd/compile/internal/test/mulconst_test.go new file mode 100644 index 0000000000..314cab32de --- /dev/null +++ b/src/cmd/compile/internal/test/mulconst_test.go @@ -0,0 +1,242 @@ +// Copyright 2020 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 test + +import "testing" + +// Benchmark multiplication of an integer by various constants. +// +// The comment above each sub-benchmark provides an example of how the +// target multiplication operation might be implemented using shift +// (multiplication by a power of 2), addition and subtraction +// operations. It is platform-dependent whether these transformations +// are actually applied. + +var ( + mulSinkI32 int32 + mulSinkI64 int64 + mulSinkU32 uint32 + mulSinkU64 uint64 +) + +func BenchmarkMulconstI32(b *testing.B) { + // 3x = 2x + x + b.Run("3", func(b *testing.B) { + x := int32(1) + for i := 0; i < b.N; i++ { + x *= 3 + } + mulSinkI32 = x + }) + // 5x = 4x + x + b.Run("5", func(b *testing.B) { + x := int32(1) + for i := 0; i < b.N; i++ { + x *= 5 + } + mulSinkI32 = x + }) + // 12x = 8x + 4x + b.Run("12", func(b *testing.B) { + x := int32(1) + for i := 0; i < b.N; i++ { + x *= 12 + } + mulSinkI32 = x + }) + // 120x = 128x - 8x + b.Run("120", func(b *testing.B) { + x := int32(1) + for i := 0; i < b.N; i++ { + x *= 120 + } + mulSinkI32 = x + }) + // -120x = 8x - 120x + b.Run("-120", func(b *testing.B) { + x := int32(1) + for i := 0; i < b.N; i++ { + x *= -120 + } + mulSinkI32 = x + }) + // 65537x = 65536x + x + b.Run("65537", func(b *testing.B) { + x := int32(1) + for i := 0; i < b.N; i++ { + x *= 65537 + } + mulSinkI32 = x + }) + // 65538x = 65536x + 2x + b.Run("65538", func(b *testing.B) { + x := int32(1) + for i := 0; i < b.N; i++ { + x *= 65538 + } + mulSinkI32 = x + }) +} + +func BenchmarkMulconstI64(b *testing.B) { + // 3x = 2x + x + b.Run("3", func(b *testing.B) { + x := int64(1) + for i := 0; i < b.N; i++ { + x *= 3 + } + mulSinkI64 = x + }) + // 5x = 4x + x + b.Run("5", func(b *testing.B) { + x := int64(1) + for i := 0; i < b.N; i++ { + x *= 5 + } + mulSinkI64 = x + }) + // 12x = 8x + 4x + b.Run("12", func(b *testing.B) { + x := int64(1) + for i := 0; i < b.N; i++ { + x *= 12 + } + mulSinkI64 = x + }) + // 120x = 128x - 8x + b.Run("120", func(b *testing.B) { + x := int64(1) + for i := 0; i < b.N; i++ { + x *= 120 + } + mulSinkI64 = x + }) + // -120x = 8x - 120x + b.Run("-120", func(b *testing.B) { + x := int64(1) + for i := 0; i < b.N; i++ { + x *= -120 + } + mulSinkI64 = x + }) + // 65537x = 65536x + x + b.Run("65537", func(b *testing.B) { + x := int64(1) + for i := 0; i < b.N; i++ { + x *= 65537 + } + mulSinkI64 = x + }) + // 65538x = 65536x + 2x + b.Run("65538", func(b *testing.B) { + x := int64(1) + for i := 0; i < b.N; i++ { + x *= 65538 + } + mulSinkI64 = x + }) +} + +func BenchmarkMulconstU32(b *testing.B) { + // 3x = 2x + x + b.Run("3", func(b *testing.B) { + x := uint32(1) + for i := 0; i < b.N; i++ { + x *= 3 + } + mulSinkU32 = x + }) + // 5x = 4x + x + b.Run("5", func(b *testing.B) { + x := uint32(1) + for i := 0; i < b.N; i++ { + x *= 5 + } + mulSinkU32 = x + }) + // 12x = 8x + 4x + b.Run("12", func(b *testing.B) { + x := uint32(1) + for i := 0; i < b.N; i++ { + x *= 12 + } + mulSinkU32 = x + }) + // 120x = 128x - 8x + b.Run("120", func(b *testing.B) { + x := uint32(1) + for i := 0; i < b.N; i++ { + x *= 120 + } + mulSinkU32 = x + }) + // 65537x = 65536x + x + b.Run("65537", func(b *testing.B) { + x := uint32(1) + for i := 0; i < b.N; i++ { + x *= 65537 + } + mulSinkU32 = x + }) + // 65538x = 65536x + 2x + b.Run("65538", func(b *testing.B) { + x := uint32(1) + for i := 0; i < b.N; i++ { + x *= 65538 + } + mulSinkU32 = x + }) +} + +func BenchmarkMulconstU64(b *testing.B) { + // 3x = 2x + x + b.Run("3", func(b *testing.B) { + x := uint64(1) + for i := 0; i < b.N; i++ { + x *= 3 + } + mulSinkU64 = x + }) + // 5x = 4x + x + b.Run("5", func(b *testing.B) { + x := uint64(1) + for i := 0; i < b.N; i++ { + x *= 5 + } + mulSinkU64 = x + }) + // 12x = 8x + 4x + b.Run("12", func(b *testing.B) { + x := uint64(1) + for i := 0; i < b.N; i++ { + x *= 12 + } + mulSinkU64 = x + }) + // 120x = 128x - 8x + b.Run("120", func(b *testing.B) { + x := uint64(1) + for i := 0; i < b.N; i++ { + x *= 120 + } + mulSinkU64 = x + }) + // 65537x = 65536x + x + b.Run("65537", func(b *testing.B) { + x := uint64(1) + for i := 0; i < b.N; i++ { + x *= 65537 + } + mulSinkU64 = x + }) + // 65538x = 65536x + 2x + b.Run("65538", func(b *testing.B) { + x := uint64(1) + for i := 0; i < b.N; i++ { + x *= 65538 + } + mulSinkU64 = x + }) +} diff --git a/test/codegen/arithmetic.go b/test/codegen/arithmetic.go index 8f25974376..9f30ec8ce4 100644 --- a/test/codegen/arithmetic.go +++ b/test/codegen/arithmetic.go @@ -71,9 +71,15 @@ func Mul_96(n int) int { // 386:`SHLL\t[$]5`,`LEAL\t\(.*\)\(.*\*2\),`,-`IMULL` // arm64:`LSL\t[$]5`,`ADD\sR[0-9]+<<1,\sR[0-9]+`,-`MUL` // arm:`SLL\t[$]5`,`ADD\sR[0-9]+<<1,\sR[0-9]+`,-`MUL` + // s390x:`SLD\t[$]5`,`SLD\t[$]6`,-`MULLD` return n * 96 } +func Mul_n120(n int) int { + // s390x:`SLD\t[$]3`,`SLD\t[$]7`,-`MULLD` + return n * -120 +} + func MulMemSrc(a []uint32, b []float32) { // 386:`IMULL\s4\([A-Z]+\),\s[A-Z]+` a[0] *= a[1] -- cgit v1.3-5-g9baa From e7c7ce646f37b260fe5a5635bc52243d28125dd8 Mon Sep 17 00:00:00 2001 From: "Paul E. Murphy" Date: Mon, 17 Aug 2020 16:14:48 -0500 Subject: cmd/compile: combine multiply/add into maddld on ppc64le/power9 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a new lowering rule to match and replace such instances with the MADDLD instruction available on power9 where possible. Likewise, this plumbs in a new ppc64 ssa opcode to house the newly generated MADDLD instructions. When testing ed25519, this reduced binary size by 936B. Similarly, MADDLD combination occcurs in a few other less obvious cases such as division by constant. Testing of golang.org/x/crypto/ed25519 shows non-trivial speedup during keygeneration: name old time/op new time/op delta KeyGeneration 65.2µs ± 0% 63.1µs ± 0% -3.19% Signing 64.3µs ± 0% 64.4µs ± 0% +0.16% Verification 147µs ± 0% 147µs ± 0% +0.11% Similarly, this test binary has shrunk by 66488B. Change-Id: I077aeda7943119b41f07e4e62e44a648f16e4ad0 Reviewed-on: https://go-review.googlesource.com/c/go/+/248723 Run-TryBot: Lynn Boger TryBot-Result: Gobot Gobot Reviewed-by: Lynn Boger --- src/cmd/compile/internal/ppc64/ssa.go | 14 ++++++++++++++ src/cmd/compile/internal/ssa/gen/PPC64.rules | 3 +++ src/cmd/compile/internal/ssa/gen/PPC64Ops.go | 2 ++ src/cmd/compile/internal/ssa/opGen.go | 16 ++++++++++++++++ src/cmd/compile/internal/ssa/rewritePPC64.go | 21 +++++++++++++++++++++ test/codegen/arithmetic.go | 12 ++++++++---- 6 files changed, 64 insertions(+), 4 deletions(-) (limited to 'test') diff --git a/src/cmd/compile/internal/ppc64/ssa.go b/src/cmd/compile/internal/ppc64/ssa.go index 0efdd710fb..4d2ad48135 100644 --- a/src/cmd/compile/internal/ppc64/ssa.go +++ b/src/cmd/compile/internal/ppc64/ssa.go @@ -601,6 +601,20 @@ func ssaGenValue(s *gc.SSAGenState, v *ssa.Value) { p.To.Type = obj.TYPE_REG p.To.Reg = v.Reg() + case ssa.OpPPC64MADDLD: + r := v.Reg() + r1 := v.Args[0].Reg() + r2 := v.Args[1].Reg() + r3 := v.Args[2].Reg() + // r = r1*r2 ± r3 + p := s.Prog(v.Op.Asm()) + p.From.Type = obj.TYPE_REG + p.From.Reg = r1 + p.Reg = r2 + p.SetFrom3(obj.Addr{Type: obj.TYPE_REG, Reg: r3}) + p.To.Type = obj.TYPE_REG + p.To.Reg = r + case ssa.OpPPC64FMADD, ssa.OpPPC64FMADDS, ssa.OpPPC64FMSUB, ssa.OpPPC64FMSUBS: r := v.Reg() r1 := v.Args[0].Reg() diff --git a/src/cmd/compile/internal/ssa/gen/PPC64.rules b/src/cmd/compile/internal/ssa/gen/PPC64.rules index fd28e10098..14942d50f9 100644 --- a/src/cmd/compile/internal/ssa/gen/PPC64.rules +++ b/src/cmd/compile/internal/ssa/gen/PPC64.rules @@ -11,6 +11,9 @@ (Sub32F ...) => (FSUBS ...) (Sub64F ...) => (FSUB ...) +// Combine 64 bit integer multiply and adds +(ADD l:(MULLD x y) z) && objabi.GOPPC64 >= 9 && l.Uses == 1 && clobber(l) => (MADDLD x y z) + (Mod16 x y) => (Mod32 (SignExt16to32 x) (SignExt16to32 y)) (Mod16u x y) => (Mod32u (ZeroExt16to32 x) (ZeroExt16to32 y)) (Mod8 x y) => (Mod32 (SignExt8to32 x) (SignExt8to32 y)) diff --git a/src/cmd/compile/internal/ssa/gen/PPC64Ops.go b/src/cmd/compile/internal/ssa/gen/PPC64Ops.go index 0261dc283b..825d0faf34 100644 --- a/src/cmd/compile/internal/ssa/gen/PPC64Ops.go +++ b/src/cmd/compile/internal/ssa/gen/PPC64Ops.go @@ -137,6 +137,7 @@ func init() { gp01 = regInfo{inputs: nil, outputs: []regMask{gp}} gp11 = regInfo{inputs: []regMask{gp | sp | sb}, outputs: []regMask{gp}} gp21 = regInfo{inputs: []regMask{gp | sp | sb, gp | sp | sb}, outputs: []regMask{gp}} + gp31 = regInfo{inputs: []regMask{gp | sp | sb, gp | sp | sb, gp | sp | sb}, outputs: []regMask{gp}} gp22 = regInfo{inputs: []regMask{gp | sp | sb, gp | sp | sb}, outputs: []regMask{gp, gp}} gp32 = regInfo{inputs: []regMask{gp | sp | sb, gp | sp | sb, gp | sp | sb}, outputs: []regMask{gp, gp}} gp1cr = regInfo{inputs: []regMask{gp | sp | sb}} @@ -179,6 +180,7 @@ func init() { {name: "MULLD", argLength: 2, reg: gp21, asm: "MULLD", typ: "Int64", commutative: true}, // arg0*arg1 (signed 64-bit) {name: "MULLW", argLength: 2, reg: gp21, asm: "MULLW", typ: "Int32", commutative: true}, // arg0*arg1 (signed 32-bit) + {name: "MADDLD", argLength: 3, reg: gp31, asm: "MADDLD", typ: "Int64"}, // (arg0*arg1)+arg2 (signed 64-bit) {name: "MULHD", argLength: 2, reg: gp21, asm: "MULHD", commutative: true}, // (arg0 * arg1) >> 64, signed {name: "MULHW", argLength: 2, reg: gp21, asm: "MULHW", commutative: true}, // (arg0 * arg1) >> 32, signed diff --git a/src/cmd/compile/internal/ssa/opGen.go b/src/cmd/compile/internal/ssa/opGen.go index df2a27368b..4cd72799e8 100644 --- a/src/cmd/compile/internal/ssa/opGen.go +++ b/src/cmd/compile/internal/ssa/opGen.go @@ -1832,6 +1832,7 @@ const ( OpPPC64FSUBS OpPPC64MULLD OpPPC64MULLW + OpPPC64MADDLD OpPPC64MULHD OpPPC64MULHW OpPPC64MULHDU @@ -24374,6 +24375,21 @@ var opcodeTable = [...]opInfo{ }, }, }, + { + name: "MADDLD", + argLen: 3, + asm: ppc64.AMADDLD, + reg: regInfo{ + inputs: []inputInfo{ + {0, 1073733630}, // SP SB R3 R4 R5 R6 R7 R8 R9 R10 R11 R12 R14 R15 R16 R17 R18 R19 R20 R21 R22 R23 R24 R25 R26 R27 R28 R29 + {1, 1073733630}, // SP SB R3 R4 R5 R6 R7 R8 R9 R10 R11 R12 R14 R15 R16 R17 R18 R19 R20 R21 R22 R23 R24 R25 R26 R27 R28 R29 + {2, 1073733630}, // SP SB R3 R4 R5 R6 R7 R8 R9 R10 R11 R12 R14 R15 R16 R17 R18 R19 R20 R21 R22 R23 R24 R25 R26 R27 R28 R29 + }, + outputs: []outputInfo{ + {0, 1073733624}, // R3 R4 R5 R6 R7 R8 R9 R10 R11 R12 R14 R15 R16 R17 R18 R19 R20 R21 R22 R23 R24 R25 R26 R27 R28 R29 + }, + }, + }, { name: "MULHD", argLen: 2, diff --git a/src/cmd/compile/internal/ssa/rewritePPC64.go b/src/cmd/compile/internal/ssa/rewritePPC64.go index 37b75cc58a..7704b80dc6 100644 --- a/src/cmd/compile/internal/ssa/rewritePPC64.go +++ b/src/cmd/compile/internal/ssa/rewritePPC64.go @@ -3852,6 +3852,27 @@ func rewriteValuePPC64_OpPPC64ADD(v *Value) bool { v_0 := v.Args[0] b := v.Block typ := &b.Func.Config.Types + // match: (ADD l:(MULLD x y) z) + // cond: objabi.GOPPC64 >= 9 && l.Uses == 1 && clobber(l) + // result: (MADDLD x y z) + for { + for _i0 := 0; _i0 <= 1; _i0, v_0, v_1 = _i0+1, v_1, v_0 { + l := v_0 + if l.Op != OpPPC64MULLD { + continue + } + y := l.Args[1] + x := l.Args[0] + z := v_1 + if !(objabi.GOPPC64 >= 9 && l.Uses == 1 && clobber(l)) { + continue + } + v.reset(OpPPC64MADDLD) + v.AddArg3(x, y, z) + return true + } + break + } // match: (ADD (SLDconst x [c]) (SRDconst x [d])) // cond: d == 64-c // result: (ROTLconst [c] x) diff --git a/test/codegen/arithmetic.go b/test/codegen/arithmetic.go index 9f30ec8ce4..45fdb68903 100644 --- a/test/codegen/arithmetic.go +++ b/test/codegen/arithmetic.go @@ -253,16 +253,20 @@ func Divisible(n1 uint, n2 int) (bool, bool, bool, bool) { // 386:"IMUL3L\t[$]-1431655765","ADDL\t[$]715827882","ROLL\t[$]31",-"DIVQ" // arm64:"MUL","ADD\t[$]3074457345618258602","ROR",-"DIV" // arm:"MUL","ADD\t[$]715827882",-".*udiv" - // ppc64:"MULLD","ADD","ROTL\t[$]63" - // ppc64le:"MULLD","ADD","ROTL\t[$]63" + // ppc64/power8:"MULLD","ADD","ROTL\t[$]63" + // ppc64le/power8:"MULLD","ADD","ROTL\t[$]63" + // ppc64/power9:"MADDLD","ROTL\t[$]63" + // ppc64le/power9:"MADDLD","ROTL\t[$]63" evenS := n2%6 == 0 // amd64:"IMULQ","ADD",-"ROLQ",-"DIVQ" // 386:"IMUL3L\t[$]678152731","ADDL\t[$]113025455",-"ROLL",-"DIVQ" // arm64:"MUL","ADD\t[$]485440633518672410",-"ROR",-"DIV" // arm:"MUL","ADD\t[$]113025455",-".*udiv" - // ppc64:"MULLD","ADD",-"ROTL" - // ppc64le:"MULLD","ADD",-"ROTL" + // ppc64/power8:"MULLD","ADD",-"ROTL" + // ppc64/power9:"MADDLD",-"ROTL" + // ppc64le/power8:"MULLD","ADD",-"ROTL" + // ppc64le/power9:"MADDLD",-"ROTL" oddS := n2%19 == 0 return evenU, oddU, evenS, oddS -- cgit v1.3-5-g9baa From ac875bc923db2b7350f244f06a06557e6fd97e05 Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Sat, 15 Aug 2020 00:44:58 +0700 Subject: cmd/compile: don't bother to declare closure inside redeclared func Fixes #17758 Change-Id: I75f5dc5be85fd8a6791ac89dfc0681be759cca36 Reviewed-on: https://go-review.googlesource.com/c/go/+/248517 Run-TryBot: Cuong Manh Le Reviewed-by: Matthew Dempsky TryBot-Result: Gobot Gobot --- src/cmd/compile/internal/gc/closure.go | 12 +++++++++++- test/fixedbugs/issue17758.go | 17 +++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 test/fixedbugs/issue17758.go (limited to 'test') diff --git a/src/cmd/compile/internal/gc/closure.go b/src/cmd/compile/internal/gc/closure.go index 04fb7d5495..23e48939b4 100644 --- a/src/cmd/compile/internal/gc/closure.go +++ b/src/cmd/compile/internal/gc/closure.go @@ -108,7 +108,17 @@ func typecheckclosure(clo *Node, top int) { xfunc.Func.Nname.Sym = closurename(Curfn) disableExport(xfunc.Func.Nname.Sym) - declare(xfunc.Func.Nname, PFUNC) + if xfunc.Func.Nname.Sym.Def != nil { + // The only case we can reach here is when the outer function was redeclared. + // In that case, don't bother to redeclare the closure. Otherwise, we will get + // a spurious error message, see #17758. While we are here, double check that + // we already reported other error. + if nsavederrors+nerrors == 0 { + Fatalf("unexpected symbol collision %v", xfunc.Func.Nname.Sym) + } + } else { + declare(xfunc.Func.Nname, PFUNC) + } xfunc = typecheck(xfunc, ctxStmt) // Type check the body now, but only if we're inside a function. diff --git a/test/fixedbugs/issue17758.go b/test/fixedbugs/issue17758.go new file mode 100644 index 0000000000..e7f2f3af91 --- /dev/null +++ b/test/fixedbugs/issue17758.go @@ -0,0 +1,17 @@ +// errorcheck + +// Copyright 2020 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 foo() { + _ = func() {} +} + +func foo() { // ERROR "foo redeclared in this block" + _ = func() {} +} + +func main() {} -- cgit v1.3-5-g9baa From 01aad9ea939fed313d5c51778485349435302ead Mon Sep 17 00:00:00 2001 From: diaxu01 Date: Fri, 5 Jun 2020 03:53:53 +0000 Subject: cmd/compile: Optimize ARM64's code with EON This patch fuses pattern '(MVN (XOR x y))' into '(EON x y)'. Change-Id: I269c98ce198d51a4945ce8bd0e1024acbd1b7609 Reviewed-on: https://go-review.googlesource.com/c/go/+/239638 Run-TryBot: Cherry Zhang TryBot-Result: Gobot Gobot Reviewed-by: Cherry Zhang --- src/cmd/compile/internal/ssa/gen/ARM64.rules | 1 + src/cmd/compile/internal/ssa/rewriteARM64.go | 12 ++++++++++++ test/codegen/bits.go | 13 +++++++++++-- 3 files changed, 24 insertions(+), 2 deletions(-) (limited to 'test') diff --git a/src/cmd/compile/internal/ssa/gen/ARM64.rules b/src/cmd/compile/internal/ssa/gen/ARM64.rules index 27959d01fc..80e8c7137b 100644 --- a/src/cmd/compile/internal/ssa/gen/ARM64.rules +++ b/src/cmd/compile/internal/ssa/gen/ARM64.rules @@ -1323,6 +1323,7 @@ (AND x (MVN y)) -> (BIC x y) (XOR x (MVN y)) -> (EON x y) (OR x (MVN y)) -> (ORN x y) +(MVN (XOR x y)) -> (EON x y) (CSEL {cc} x (MOVDconst [0]) flag) -> (CSEL0 {cc} x flag) (CSEL {cc} (MOVDconst [0]) y flag) -> (CSEL0 {arm64Negate(cc.(Op))} y flag) (SUB x (SUB y z)) -> (SUB (ADD x z) y) diff --git a/src/cmd/compile/internal/ssa/rewriteARM64.go b/src/cmd/compile/internal/ssa/rewriteARM64.go index 023d9908c2..842eddbf4a 100644 --- a/src/cmd/compile/internal/ssa/rewriteARM64.go +++ b/src/cmd/compile/internal/ssa/rewriteARM64.go @@ -14593,6 +14593,18 @@ func rewriteValueARM64_OpARM64MULW(v *Value) bool { } func rewriteValueARM64_OpARM64MVN(v *Value) bool { v_0 := v.Args[0] + // match: (MVN (XOR x y)) + // result: (EON x y) + for { + if v_0.Op != OpARM64XOR { + break + } + y := v_0.Args[1] + x := v_0.Args[0] + v.reset(OpARM64EON) + v.AddArg2(x, y) + return true + } // match: (MVN (MOVDconst [c])) // result: (MOVDconst [^c]) for { diff --git a/test/codegen/bits.go b/test/codegen/bits.go index 0a5428b55a..398dd84e9e 100644 --- a/test/codegen/bits.go +++ b/test/codegen/bits.go @@ -310,9 +310,18 @@ func op_bic(x, y uint32) uint32 { return x &^ y } -func op_eon(x, y uint32) uint32 { +func op_eon(x, y, z uint32, a []uint32, n, m uint64) uint64 { + // arm64:`EON\t`,-`EOR`,-`MVN` + a[0] = x ^ (y ^ 0xffffffff) + + // arm64:`EON\t`,-`EOR`,-`MVN` + a[1] = ^(y ^ z) + // arm64:`EON\t`,-`XOR` - return x ^ ^y + a[2] = x ^ ^z + + // arm64:`EON\t`,-`EOR`,-`MVN` + return n ^ (m ^ 0xffffffffffffffff) } func op_orn(x, y uint32) uint32 { -- cgit v1.3-5-g9baa From e94544cf012535da6b3c9e735bc4026e2db1c99c Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Wed, 19 Aug 2020 21:39:12 -0700 Subject: cmd/compile: fix checkptr handling of &^ checkptr has code to recognize &^ expressions, but it didn't take into account that "p &^ x" gets rewritten to "p & ^x" during walk, which resulted in false positive diagnostics. This CL changes walkexpr to mark OANDNOT expressions with Implicit when they're rewritten to OAND, so that walkCheckPtrArithmetic can still recognize them later. It would be slightly more idiomatic to instead mark the OBITNOT expression as Implicit (as it's a compiler-generated Node), but the OBITNOT expression might get constant folded. It's not worth the extra complexity/subtlety of relying on n.Right.Orig, so we set Implicit on the OAND node instead. To atone for this transgression, I add documentation for nodeImplicit. Fixes #40917. Change-Id: I386304171ad299c530e151e5924f179e9a5fd5b8 Reviewed-on: https://go-review.googlesource.com/c/go/+/249477 Run-TryBot: Matthew Dempsky TryBot-Result: Gobot Gobot Reviewed-by: Keith Randall Reviewed-by: Cuong Manh Le --- src/cmd/compile/internal/gc/syntax.go | 4 ++-- src/cmd/compile/internal/gc/walk.go | 7 ++++++- src/runtime/checkptr_test.go | 1 + src/runtime/testdata/testprog/checkptr.go | 8 ++++++++ test/fixedbugs/issue40917.go | 23 +++++++++++++++++++++++ 5 files changed, 40 insertions(+), 3 deletions(-) create mode 100644 test/fixedbugs/issue40917.go (limited to 'test') diff --git a/src/cmd/compile/internal/gc/syntax.go b/src/cmd/compile/internal/gc/syntax.go index b658410c53..47e5e59156 100644 --- a/src/cmd/compile/internal/gc/syntax.go +++ b/src/cmd/compile/internal/gc/syntax.go @@ -141,8 +141,8 @@ const ( nodeInitorder, _ // tracks state during init1; two bits _, _ // second nodeInitorder bit _, nodeHasBreak - _, nodeNoInline // used internally by inliner to indicate that a function call should not be inlined; set for OCALLFUNC and OCALLMETH only - _, nodeImplicit + _, nodeNoInline // used internally by inliner to indicate that a function call should not be inlined; set for OCALLFUNC and OCALLMETH only + _, nodeImplicit // implicit OADDR or ODEREF; ++/-- statement represented as OASOP; or ANDNOT lowered to OAND _, nodeIsDDD // is the argument variadic _, nodeDiag // already printed error about this _, nodeColas // OAS resulting from := diff --git a/src/cmd/compile/internal/gc/walk.go b/src/cmd/compile/internal/gc/walk.go index 8ae3d9a5c7..74ed0411bd 100644 --- a/src/cmd/compile/internal/gc/walk.go +++ b/src/cmd/compile/internal/gc/walk.go @@ -973,6 +973,7 @@ opswitch: case OANDNOT: n.Left = walkexpr(n.Left, init) n.Op = OAND + n.SetImplicit(true) // for walkCheckPtrArithmetic n.Right = nod(OBITNOT, n.Right, nil) n.Right = typecheck(n.Right, ctxExpr) n.Right = walkexpr(n.Right, init) @@ -4003,8 +4004,12 @@ func walkCheckPtrArithmetic(n *Node, init *Nodes) *Node { case OADD: walk(n.Left) walk(n.Right) - case OSUB, OANDNOT: + case OSUB: walk(n.Left) + case OAND: + if n.Implicit() { // was OANDNOT + walk(n.Left) + } case OCONVNOP: if n.Left.Type.Etype == TUNSAFEPTR { n.Left = cheapexpr(n.Left, init) diff --git a/src/runtime/checkptr_test.go b/src/runtime/checkptr_test.go index 8ab8a4937c..194cc1243a 100644 --- a/src/runtime/checkptr_test.go +++ b/src/runtime/checkptr_test.go @@ -27,6 +27,7 @@ func TestCheckPtr(t *testing.T) { {"CheckPtrAlignmentPtr", "fatal error: checkptr: misaligned pointer conversion\n"}, {"CheckPtrAlignmentNoPtr", ""}, {"CheckPtrArithmetic", "fatal error: checkptr: pointer arithmetic result points to invalid allocation\n"}, + {"CheckPtrArithmetic2", "fatal error: checkptr: pointer arithmetic result points to invalid allocation\n"}, {"CheckPtrSize", "fatal error: checkptr: converted pointer straddles multiple allocations\n"}, {"CheckPtrSmall", "fatal error: checkptr: pointer arithmetic computed bad pointer value\n"}, } diff --git a/src/runtime/testdata/testprog/checkptr.go b/src/runtime/testdata/testprog/checkptr.go index 45e6fb1aa5..e0a2794f4c 100644 --- a/src/runtime/testdata/testprog/checkptr.go +++ b/src/runtime/testdata/testprog/checkptr.go @@ -10,6 +10,7 @@ func init() { register("CheckPtrAlignmentNoPtr", CheckPtrAlignmentNoPtr) register("CheckPtrAlignmentPtr", CheckPtrAlignmentPtr) register("CheckPtrArithmetic", CheckPtrArithmetic) + register("CheckPtrArithmetic2", CheckPtrArithmetic2) register("CheckPtrSize", CheckPtrSize) register("CheckPtrSmall", CheckPtrSmall) } @@ -32,6 +33,13 @@ func CheckPtrArithmetic() { sink2 = (*int)(unsafe.Pointer(i)) } +func CheckPtrArithmetic2() { + var x [2]int64 + p := unsafe.Pointer(&x[1]) + var one uintptr = 1 + sink2 = unsafe.Pointer(uintptr(p) & ^one) +} + func CheckPtrSize() { p := new(int64) sink2 = p diff --git a/test/fixedbugs/issue40917.go b/test/fixedbugs/issue40917.go new file mode 100644 index 0000000000..2128be5eca --- /dev/null +++ b/test/fixedbugs/issue40917.go @@ -0,0 +1,23 @@ +// run -gcflags=-d=checkptr + +// Copyright 2020 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 "unsafe" + +func main() { + var x [2]uint64 + a := unsafe.Pointer(&x[1]) + + b := a + b = unsafe.Pointer(uintptr(b) + 2) + b = unsafe.Pointer(uintptr(b) - 1) + b = unsafe.Pointer(uintptr(b) &^ 1) + + if a != b { + panic("pointer arithmetic failed") + } +} -- cgit v1.3-5-g9baa