Skip to content

Commit

Permalink
cmd/compile: port callnew to ssa conversion
Browse files Browse the repository at this point in the history
This is part of a general effort to shrink walk.
In an ideal world, we'd have an SSA op for allocation,
but we don't yet have a good mechanism for introducing
function calling during SSA compilation.
In the meantime, SSA conversion is a better place for it.

This also makes it easier to introduce new optimizations;
instead of doing the typecheck walk dance,
we can simply write what we want the backend to do.

I introduced a new opcode in this change because:

(a) It avoids a class of bugs involving correctly detecting
    whether this ONEW is a "before walk" ONEW or an "after walk" ONEW.
    It also means that using ONEW or ONEWOBJ in the wrong context
    will generally result in a faster failure.
(b) Opcodes are cheap.
(c) It provides a better place to put documentation.

This change also is also marginally more performant:

name        old alloc/op      new alloc/op      delta
Template         39.1MB ± 0%       39.0MB ± 0%  -0.14%  (p=0.008 n=5+5)
Unicode          28.4MB ± 0%       28.4MB ± 0%    ~     (p=0.421 n=5+5)
GoTypes           132MB ± 0%        132MB ± 0%  -0.23%  (p=0.008 n=5+5)
Compiler          608MB ± 0%        607MB ± 0%  -0.25%  (p=0.008 n=5+5)
SSA              2.04GB ± 0%       2.04GB ± 0%  -0.01%  (p=0.008 n=5+5)
Flate            24.4MB ± 0%       24.3MB ± 0%  -0.13%  (p=0.008 n=5+5)
GoParser         29.3MB ± 0%       29.1MB ± 0%  -0.54%  (p=0.008 n=5+5)
Reflect          84.8MB ± 0%       84.7MB ± 0%  -0.21%  (p=0.008 n=5+5)
Tar              36.7MB ± 0%       36.6MB ± 0%  -0.10%  (p=0.008 n=5+5)
XML              48.7MB ± 0%       48.6MB ± 0%  -0.24%  (p=0.008 n=5+5)
[Geo mean]       85.0MB            84.8MB       -0.19%

name        old allocs/op     new allocs/op     delta
Template           383k ± 0%         382k ± 0%  -0.26%  (p=0.008 n=5+5)
Unicode            341k ± 0%         341k ± 0%    ~     (p=0.579 n=5+5)
GoTypes           1.37M ± 0%        1.36M ± 0%  -0.39%  (p=0.008 n=5+5)
Compiler          5.59M ± 0%        5.56M ± 0%  -0.49%  (p=0.008 n=5+5)
SSA               16.9M ± 0%        16.9M ± 0%  -0.03%  (p=0.008 n=5+5)
Flate              238k ± 0%         238k ± 0%  -0.23%  (p=0.008 n=5+5)
GoParser           306k ± 0%         303k ± 0%  -0.93%  (p=0.008 n=5+5)
Reflect            990k ± 0%         987k ± 0%  -0.33%  (p=0.008 n=5+5)
Tar                356k ± 0%         355k ± 0%  -0.20%  (p=0.008 n=5+5)
XML                444k ± 0%         442k ± 0%  -0.45%  (p=0.008 n=5+5)
[Geo mean]         848k              845k       -0.33%

Change-Id: I2c36003a7cbf71b53857b7de734852b698f49310
Reviewed-on: https://go-review.googlesource.com/c/go/+/167957
Run-TryBot: Josh Bleecher Snyder <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>
  • Loading branch information
josharian committed Mar 20, 2019
1 parent fd270d8 commit 23b476a
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 20 deletions.
4 changes: 3 additions & 1 deletion src/cmd/compile/internal/gc/go.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ var (
growslice,
msanread,
msanwrite,
newobject,
newproc,
panicdivide,
panicshift,
Expand All @@ -312,7 +313,8 @@ var (
typedmemclr,
typedmemmove,
Udiv,
writeBarrier *obj.LSym
writeBarrier,
zerobaseSym *obj.LSym

BoundsCheckFunc [ssa.BoundsKindCount]*obj.LSym
ExtendCheckFunc [ssa.BoundsKindCount]*obj.LSym
Expand Down
4 changes: 2 additions & 2 deletions src/cmd/compile/internal/gc/op_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions src/cmd/compile/internal/gc/ssa.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ func initssaconfig() {
growslice = sysfunc("growslice")
msanread = sysfunc("msanread")
msanwrite = sysfunc("msanwrite")
newobject = sysfunc("newobject")
newproc = sysfunc("newproc")
panicdivide = sysfunc("panicdivide")
panicdottypeE = sysfunc("panicdottypeE")
Expand All @@ -94,6 +95,8 @@ func initssaconfig() {
typedmemmove = sysfunc("typedmemmove")
Udiv = sysvar("udiv") // asm func with special ABI
writeBarrier = sysvar("writeBarrier") // struct { bool; ... }
zerobaseSym = sysvar("zerobase")

if thearch.LinkArch.Family == sys.Wasm {
BoundsCheckFunc[ssa.BoundsIndex] = sysvar("goPanicIndex")
BoundsCheckFunc[ssa.BoundsIndexU] = sysvar("goPanicIndexU")
Expand Down Expand Up @@ -2453,6 +2456,14 @@ func (s *state) expr(n *Node) *ssa.Value {
}
return s.zeroVal(n.Type)

case ONEWOBJ:
if n.Type.Elem().Size() == 0 {
return s.newValue1A(ssa.OpAddr, n.Type, zerobaseSym, s.sb)
}
typ := s.expr(n.Left)
vv := s.rtcall(newobject, true, []*types.Type{n.Type}, typ)
return vv[0]

default:
s.Fatalf("unhandled expr %v", n.Op)
return nil
Expand Down
3 changes: 2 additions & 1 deletion src/cmd/compile/internal/gc/syntax.go
Original file line number Diff line number Diff line change
Expand Up @@ -668,7 +668,8 @@ const (
ORSH // Left >> Right
OAND // Left & Right
OANDNOT // Left &^ Right
ONEW // new(Left)
ONEW // new(Left); corresponds to calls to new in source code
ONEWOBJ // runtime.newobject(n.Type); introduced by walk; Left is type descriptor
ONOT // !Left
OBITNOT // ^Left
OPLUS // +Left
Expand Down
22 changes: 6 additions & 16 deletions src/cmd/compile/internal/gc/walk.go
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ opswitch:
Dump("walk", n)
Fatalf("walkexpr: switch 1 unknown op %+S", n)

case ONONAME, OINDREGSP, OEMPTY, OGETG:
case ONONAME, OINDREGSP, OEMPTY, OGETG, ONEWOBJ:

case OTYPE, ONAME, OLITERAL:
// TODO(mdempsky): Just return n; see discussion on CL 38655.
Expand Down Expand Up @@ -1944,21 +1944,11 @@ func callnew(t *types.Type) *Node {
yyerror("%v is go:notinheap; heap allocation disallowed", t)
}
dowidth(t)

if t.Size() == 0 {
// Return &runtime.zerobase if we know that the requested size is 0.
// This is what runtime.mallocgc would return.
z := newname(Runtimepkg.Lookup("zerobase"))
z.SetClass(PEXTERN)
z.Type = t
return typecheck(nod(OADDR, z, nil), ctxExpr)
}

fn := syslook("newobject")
fn = substArgTypes(fn, t)
v := mkcall1(fn, types.NewPtr(t), nil, typename(t))
v.SetNonNil(true)
return v
n := nod(ONEWOBJ, typename(t), nil)
n.Type = types.NewPtr(t)
n.SetTypecheck(1)
n.SetNonNil(true)
return n
}

// isReflectHeaderDataField reports whether l is an expression p.Data
Expand Down

0 comments on commit 23b476a

Please sign in to comment.