From 3c1a4c1902711c16489ed0c3506df97439ffbd85 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Tue, 19 Apr 2016 21:06:53 -0700 Subject: [PATCH] cmd/compile: don't nilcheck newobject and return values from mapaccess{1,2} They are guaranteed to be non-nil, no point in inserting nil checks for them. Fixes #15390 Change-Id: I3b9a0f2319affc2139dcc446d0a56c6785ae5a86 Reviewed-on: https://go-review.googlesource.com/22291 Reviewed-by: Josh Bleecher Snyder --- src/cmd/compile/internal/gc/cgen.go | 24 +++++++++++++++---- src/cmd/compile/internal/gc/fmt.go | 6 +++++ src/cmd/compile/internal/gc/ssa.go | 30 ++++++++++++++---------- src/cmd/compile/internal/gc/syntax.go | 1 + src/cmd/compile/internal/gc/walk.go | 9 +++---- src/cmd/compile/internal/ssa/nilcheck.go | 2 -- test/nilptr3.go | 21 +++++++++++++++++ test/nilptr3_ssa.go | 21 +++++++++++++++++ 8 files changed, 91 insertions(+), 23 deletions(-) diff --git a/src/cmd/compile/internal/gc/cgen.go b/src/cmd/compile/internal/gc/cgen.go index 5c5bedaa314..a9393a6d9e1 100644 --- a/src/cmd/compile/internal/gc/cgen.go +++ b/src/cmd/compile/internal/gc/cgen.go @@ -978,7 +978,11 @@ func Agenr(n *Node, a *Node, res *Node) { case OIND: Cgenr(n.Left, a, res) - Cgen_checknil(a) + if !n.Left.NonNil { + Cgen_checknil(a) + } else if Debug_checknil != 0 && n.Lineno > 1 { + Warnl(n.Lineno, "removed nil check") + } case OINDEX: if Ctxt.Arch.Family == sys.ARM { @@ -1587,7 +1591,11 @@ func Agen(n *Node, res *Node) { case OIND: Cgen(nl, res) - Cgen_checknil(res) + if !nl.NonNil { + Cgen_checknil(res) + } else if Debug_checknil != 0 && n.Lineno > 1 { + Warnl(n.Lineno, "removed nil check") + } case ODOT: Agen(nl, res) @@ -1597,7 +1605,11 @@ func Agen(n *Node, res *Node) { case ODOTPTR: Cgen(nl, res) - Cgen_checknil(res) + if !nl.NonNil { + Cgen_checknil(res) + } else if Debug_checknil != 0 && n.Lineno > 1 { + Warnl(n.Lineno, "removed nil check") + } if n.Xoffset != 0 { addOffset(res, n.Xoffset) } @@ -1658,7 +1670,11 @@ func Igen(n *Node, a *Node, res *Node) { case ODOTPTR: Cgenr(n.Left, a, res) - Cgen_checknil(a) + if !n.Left.NonNil { + Cgen_checknil(a) + } else if Debug_checknil != 0 && n.Lineno > 1 { + Warnl(n.Lineno, "removed nil check") + } a.Op = OINDREG a.Xoffset += n.Xoffset a.Type = n.Type diff --git a/src/cmd/compile/internal/gc/fmt.go b/src/cmd/compile/internal/gc/fmt.go index bfb031aac58..e5977c0905f 100644 --- a/src/cmd/compile/internal/gc/fmt.go +++ b/src/cmd/compile/internal/gc/fmt.go @@ -319,6 +319,12 @@ func Jconv(n *Node, flag FmtFlag) string { if n.Assigned { buf.WriteString(" assigned") } + if n.Bounded { + buf.WriteString(" bounded") + } + if n.NonNil { + buf.WriteString(" nonnil") + } if c == 0 && n.Used { fmt.Fprintf(&buf, " used(%v)", n.Used) diff --git a/src/cmd/compile/internal/gc/ssa.go b/src/cmd/compile/internal/gc/ssa.go index ad665fbfbc0..218f720a61f 100644 --- a/src/cmd/compile/internal/gc/ssa.go +++ b/src/cmd/compile/internal/gc/ssa.go @@ -1938,8 +1938,7 @@ func (s *state) expr(n *Node) *ssa.Value { return s.newValue2(ssa.OpLoad, n.Type, addr, s.mem()) case OIND: - p := s.expr(n.Left) - s.nilCheck(p) + p := s.exprPtr(n.Left, false, n.Lineno) return s.newValue2(ssa.OpLoad, n.Type, p, s.mem()) case ODOT: @@ -1952,8 +1951,7 @@ func (s *state) expr(n *Node) *ssa.Value { return s.newValue2(ssa.OpLoad, n.Type, p, s.mem()) case ODOTPTR: - p := s.expr(n.Left) - s.nilCheck(p) + p := s.exprPtr(n.Left, false, n.Lineno) p = s.newValue1I(ssa.OpOffPtr, p.Type, n.Xoffset, p) return s.newValue2(ssa.OpLoad, n.Type, p, s.mem()) @@ -2778,19 +2776,12 @@ func (s *state) addr(n *Node, bounded bool) *ssa.Value { return s.newValue2(ssa.OpPtrIndex, Ptrto(n.Left.Type.Elem()), a, i) } case OIND: - p := s.expr(n.Left) - if !bounded { - s.nilCheck(p) - } - return p + return s.exprPtr(n.Left, bounded, n.Lineno) case ODOT: p := s.addr(n.Left, bounded) return s.newValue1I(ssa.OpOffPtr, t, n.Xoffset, p) case ODOTPTR: - p := s.expr(n.Left) - if !bounded { - s.nilCheck(p) - } + p := s.exprPtr(n.Left, bounded, n.Lineno) return s.newValue1I(ssa.OpOffPtr, t, n.Xoffset, p) case OCLOSUREVAR: return s.newValue1I(ssa.OpOffPtr, t, n.Xoffset, @@ -2892,6 +2883,19 @@ func canSSAType(t *Type) bool { } } +// exprPtr evaluates n to a pointer and nil-checks it. +func (s *state) exprPtr(n *Node, bounded bool, lineno int32) *ssa.Value { + p := s.expr(n) + if bounded || n.NonNil { + if s.f.Config.Debug_checknil() && lineno > 1 { + s.f.Config.Warnl(lineno, "removed nil check") + } + return p + } + s.nilCheck(p) + return p +} + // nilCheck generates nil pointer checking code. // Starts a new block on return, unless nil checks are disabled. // Used only for automatically inserted nil checks, diff --git a/src/cmd/compile/internal/gc/syntax.go b/src/cmd/compile/internal/gc/syntax.go index 2f3b98a8ef8..8a675ac1579 100644 --- a/src/cmd/compile/internal/gc/syntax.go +++ b/src/cmd/compile/internal/gc/syntax.go @@ -54,6 +54,7 @@ type Node struct { Addable bool // addressable Etype EType // op for OASOP, etype for OTYPE, exclam for export, 6g saved reg, ChanDir for OTCHAN Bounded bool // bounds check unnecessary + NonNil bool // guaranteed to be non-nil Class Class // PPARAM, PAUTO, PEXTERN, etc Embedded uint8 // ODCLFIELD embedded type Colas bool // OAS resulting from := diff --git a/src/cmd/compile/internal/gc/walk.go b/src/cmd/compile/internal/gc/walk.go index 0e74365c76a..27ff045028a 100644 --- a/src/cmd/compile/internal/gc/walk.go +++ b/src/cmd/compile/internal/gc/walk.go @@ -886,6 +886,7 @@ opswitch: if !isblank(a) { var_ := temp(Ptrto(t.Val())) var_.Typecheck = 1 + var_.NonNil = true // mapaccess always returns a non-nil pointer n.List.SetIndex(0, var_) n = walkexpr(n, init) init.Append(n) @@ -895,8 +896,6 @@ opswitch: n = typecheck(n, Etop) n = walkexpr(n, init) - // TODO: ptr is always non-nil, so disable nil check for this OIND op. - case ODELETE: init.AppendNodes(&n.Ninit) map_ := n.List.First() @@ -1224,7 +1223,6 @@ opswitch: // standard version takes key by reference. // orderexpr made sure key is addressable. key = Nod(OADDR, n.Right, nil) - p = "mapaccess1" } @@ -1235,6 +1233,7 @@ opswitch: z := zeroaddr(w) n = mkcall1(mapfn(p, t), Ptrto(t.Val()), init, typename(t), n.Left, key, z) } + n.NonNil = true // mapaccess always returns a non-nil pointer n = Nod(OIND, n, nil) n.Type = t.Val() n.Typecheck = 1 @@ -2015,7 +2014,9 @@ func callnew(t *Type) *Node { dowidth(t) fn := syslook("newobject") fn = substArgTypes(fn, t) - return mkcall1(fn, Ptrto(t), nil, typename(t)) + v := mkcall1(fn, Ptrto(t), nil, typename(t)) + v.NonNil = true + return v } func iscallret(n *Node) bool { diff --git a/src/cmd/compile/internal/ssa/nilcheck.go b/src/cmd/compile/internal/ssa/nilcheck.go index 753e48aad57..62eb0c8ea61 100644 --- a/src/cmd/compile/internal/ssa/nilcheck.go +++ b/src/cmd/compile/internal/ssa/nilcheck.go @@ -4,8 +4,6 @@ package ssa -// TODO: return value from newobject/newarray is non-nil. - // nilcheckelim eliminates unnecessary nil checks. func nilcheckelim(f *Func) { // A nil check is redundant if the same nil check was successful in a diff --git a/test/nilptr3.go b/test/nilptr3.go index 817d2aec74a..1bec833fe32 100644 --- a/test/nilptr3.go +++ b/test/nilptr3.go @@ -193,3 +193,24 @@ func f4(x *[10]int) { x = y _ = &x[9] // ERROR "removed repeated nil check" } + +func m1(m map[int][80]byte) byte { + v := m[3] // ERROR "removed nil check" + return v[5] +} +func m2(m map[int][800]byte) byte { + v := m[3] // ERROR "removed nil check" + return v[5] +} +func m3(m map[int][80]byte) (byte, bool) { + v, ok := m[3] // ERROR "removed nil check" + return v[5], ok +} +func m4(m map[int][800]byte) (byte, bool) { + v, ok := m[3] // ERROR "removed nil check" + return v[5], ok +} +func p1() byte { + p := new([100]byte) + return p[5] // ERROR "removed nil check" +} diff --git a/test/nilptr3_ssa.go b/test/nilptr3_ssa.go index ba60a64602b..6eefbac7d80 100644 --- a/test/nilptr3_ssa.go +++ b/test/nilptr3_ssa.go @@ -207,3 +207,24 @@ func f6(p, q *T) { x := *p // ERROR "removed nil check" *q = x // ERROR "removed nil check" } + +func m1(m map[int][80]byte) byte { + v := m[3] // ERROR "removed nil check" + return v[5] +} +func m2(m map[int][800]byte) byte { + v := m[3] // ERROR "removed nil check" + return v[5] +} +func m3(m map[int][80]byte) (byte, bool) { + v, ok := m[3] // ERROR "removed nil check" + return v[5], ok +} +func m4(m map[int][800]byte) (byte, bool) { + v, ok := m[3] // ERROR "removed nil check" + return v[5], ok +} +func p1() byte { + p := new([100]byte) + return p[5] // ERROR "removed nil check" +}