build: enable framepointer mode by default

This has a minor performance cost, but far less than is being gained by SSA.
As an experiment, enable it during the Go 1.7 beta.
Having frame pointers on by default makes Linux's perf, Intel VTune,
and other profilers much more useful, because it lets them gather a
stack trace efficiently on profiling events.
(It doesn't help us that much, since when we walk the stack we usually
need to look up PC-specific information as well.)

Fixes #15840.

Change-Id: I4efd38412a0de4a9c87b1b6e5d11c301e63f1a2a
Reviewed-on: https://go-review.googlesource.com/23451
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/cmd/compile/internal/amd64/galign.go b/src/cmd/compile/internal/amd64/galign.go
index 461ef2ada1bb3299222f5278baa44925378985a7..42915340a06bf8d6067114919226aaf90ef3e4a4 100644 (file)
--- a/src/cmd/compile/internal/amd64/galign.go
+++ b/src/cmd/compile/internal/amd64/galign.go
@@ -25,18 +25,16 @@ func betypeinit() {
                cmpptr = x86.ACMPL
        }
 
-       if gc.Ctxt.Flag_dynlink {
-               gc.Thearch.ReservedRegs = append(gc.Thearch.ReservedRegs, x86.REG_R15)
+       if gc.Ctxt.Flag_dynlink || obj.Getgoos() == "nacl" {
+               resvd = append(resvd, x86.REG_R15)
        }
-}
-
-func Main() {
-       if obj.Getgoos() == "nacl" {
-               resvd = append(resvd, x86.REG_BP, x86.REG_R15)
-       } else if obj.Framepointer_enabled != 0 {
+       if gc.Ctxt.Framepointer_enabled || obj.Getgoos() == "nacl" {
                resvd = append(resvd, x86.REG_BP)
        }
+       gc.Thearch.ReservedRegs = resvd
+}
 
+func Main() {
        gc.Thearch.LinkArch = &x86.Linkamd64
        if obj.Getgoarch() == "amd64p32" {
                gc.Thearch.LinkArch = &x86.Linkamd64p32
@@ -51,7 +49,6 @@ func Main() {
        gc.Thearch.FREGMIN = x86.REG_X0
        gc.Thearch.FREGMAX = x86.REG_X15
        gc.Thearch.MAXWIDTH = 1 << 50
-       gc.Thearch.ReservedRegs = resvd
 
        gc.Thearch.AddIndex = addindex
        gc.Thearch.Betypeinit = betypeinit

diff --git a/src/cmd/compile/internal/amd64/reg.go b/src/cmd/compile/internal/amd64/reg.go
index 764f5c3a9e63802b1f9370ec5cd56357c92d71e2..77720c855f13f17156827530ef5456102fdc5ea4 100644 (file)
--- a/src/cmd/compile/internal/amd64/reg.go
+++ b/src/cmd/compile/internal/amd64/reg.go
@@ -32,7 +32,6 @@ package amd64
 
 import (
        "cmd/compile/internal/gc"
-       "cmd/internal/obj"
        "cmd/internal/obj/x86"
 )
 
@@ -121,7 +120,7 @@ func BtoR(b uint64) int {
        b &= 0xffff
        if gc.Nacl {
                b &^= (1<<(x86.REG_BP-x86.REG_AX) | 1<<(x86.REG_R15-x86.REG_AX))
-       } else if obj.Framepointer_enabled != 0 {
+       } else if gc.Ctxt.Framepointer_enabled {
                // BP is part of the calling convention if framepointer_enabled.
                b &^= (1 << (x86.REG_BP - x86.REG_AX))
        }

diff --git a/src/cmd/compile/internal/ssa/regalloc.go b/src/cmd/compile/internal/ssa/regalloc.go
index bd405225746e70bf9c06d3ca981f3a17cf29cf7a..1eecd49c40cf3a322628f5a4419667fd32917ef8 100644 (file)
--- a/src/cmd/compile/internal/ssa/regalloc.go
+++ b/src/cmd/compile/internal/ssa/regalloc.go
@@ -106,7 +106,6 @@
 package ssa
 
 import (
-       "cmd/internal/obj"
        "fmt"
        "unsafe"
 )
@@ -456,7 +455,7 @@ func (s *regAllocState) init(f *Func) {
        s.allocatable = regMask(1)<<s.numRegs - 1
        s.allocatable &^= 1 << s.SPReg
        s.allocatable &^= 1 << s.SBReg
-       if obj.Framepointer_enabled != 0 {
+       if s.f.Config.ctxt.Framepointer_enabled {
                s.allocatable &^= 1 << 5 // BP
        }
        if s.f.Config.ctxt.Flag_dynlink {

diff --git a/src/cmd/internal/obj/go.go b/src/cmd/internal/obj/go.go
index 484bb472d09e167cf85054205369fbc8d134a25d..1852dc74f63b7d4e0f014932d38c9d8f87316210 100644 (file)
--- a/src/cmd/internal/obj/go.go
+++ b/src/cmd/internal/obj/go.go
@@ -13,7 +13,7 @@ import (
 // go-specific code shared across loaders (5l, 6l, 8l).
 
 var (
-       Framepointer_enabled int
+       framepointer_enabled int
        Fieldtrack_enabled   int
 )
 
@@ -26,14 +26,21 @@ var exper = []struct {
        val  *int
 }{
        {"fieldtrack", &Fieldtrack_enabled},
-       {"framepointer", &Framepointer_enabled},
+       {"framepointer", &framepointer_enabled},
 }
 
 func addexp(s string) {
+       // Could do general integer parsing here, but the runtime copy doesn't yet.
+       v := 1
+       name := s
+       if len(name) > 2 && name[:2] == "no" {
+               v = 0
+               name = name[2:]
+       }
        for i := 0; i < len(exper); i++ {
-               if exper[i].name == s {
+               if exper[i].name == name {
                        if exper[i].val != nil {
-                               *exper[i].val = 1
+                               *exper[i].val = v
                        }
                        return
                }
@@ -44,6 +51,7 @@ func addexp(s string) {
 }
 
 func init() {
+       framepointer_enabled = 1 // default
        for _, f := range strings.Split(goexperiment, ",") {
                if f != "" {
                        addexp(f)
@@ -51,6 +59,10 @@ func init() {
        }
 }
 
+func Framepointer_enabled(goos, goarch string) bool {
+       return framepointer_enabled != 0 && goarch == "amd64" && goos != "nacl"
+}
+
 func Nopout(p *Prog) {
        p.As = ANOP
        p.Scond = 0

diff --git a/src/cmd/internal/obj/link.go b/src/cmd/internal/obj/link.go
index eaf702533a141fea9d8ec0dc13983cf5a5ccb2fa..b6861f4c1e572c7b112ea61c2c685928688c1c1d 100644 (file)
--- a/src/cmd/internal/obj/link.go
+++ b/src/cmd/internal/obj/link.go
@@ -664,6 +664,8 @@ type Link struct {
        Etextp        *LSym
        Errors        int
 
+       Framepointer_enabled bool
+
        // state for writing objects
        Text []*LSym
        Data []*LSym

diff --git a/src/cmd/internal/obj/sym.go b/src/cmd/internal/obj/sym.go
index 6f3542b3d4f42102ad6453088b63acc814783264..e974ca8c8af30602c7b39780fb46d2af20ff9866 100644 (file)
--- a/src/cmd/internal/obj/sym.go
+++ b/src/cmd/internal/obj/sym.go
@@ -106,6 +106,7 @@ func Linknew(arch *LinkArch) *Link {
        }
 
        ctxt.Flag_optimize = true
+       ctxt.Framepointer_enabled = Framepointer_enabled(Getgoos(), arch.Name)
        return ctxt
 }
 

diff --git a/src/cmd/internal/obj/x86/asm6.go b/src/cmd/internal/obj/x86/asm6.go
index 9230c9fdacb1da362df5b11f4bde4eacd3137f0e..414a4d34a558c58d0c90bb8fce38761f3e5f2192 100644 (file)
--- a/src/cmd/internal/obj/x86/asm6.go
+++ b/src/cmd/internal/obj/x86/asm6.go
@@ -3765,7 +3765,7 @@ func doasm(ctxt *obj.Link, p *obj.Prog) {
                                        ctxt.Diag("directly calling duff when dynamically linking Go")
                                }
 
-                               if obj.Framepointer_enabled != 0 && yt.zcase == Zcallduff && p.Mode == 64 {
+                               if ctxt.Framepointer_enabled && yt.zcase == Zcallduff && p.Mode == 64 {
                                        // Maintain BP around call, since duffcopy/duffzero can't do it
                                        // (the call jumps into the middle of the function).
                                        // This makes it possible to see call sites for duffcopy/duffzero in
@@ -3784,7 +3784,7 @@ func doasm(ctxt *obj.Link, p *obj.Prog) {
                                r.Siz = 4
                                ctxt.AsmBuf.PutInt32(0)
 
-                               if obj.Framepointer_enabled != 0 && yt.zcase == Zcallduff && p.Mode == 64 {
+                               if ctxt.Framepointer_enabled && yt.zcase == Zcallduff && p.Mode == 64 {
                                        // Pop BP pushed above.
                                        // MOVQ 0(BP), BP
                                        ctxt.AsmBuf.Put(bpduff2)

diff --git a/src/cmd/internal/obj/x86/obj6.go b/src/cmd/internal/obj/x86/obj6.go
index 0f1f28d36d6d940d0e984380c1c32d9b931f3d64..5dad0bbb982616d921cd15d254d7544f2e298eb4 100644 (file)
--- a/src/cmd/internal/obj/x86/obj6.go
+++ b/src/cmd/internal/obj/x86/obj6.go
@@ -610,7 +610,7 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym) {
        }
 
        var bpsize int
-       if p.Mode == 64 && obj.Framepointer_enabled != 0 && autoffset > 0 && p.From3.Offset&obj.NOFRAME == 0 {
+       if p.Mode == 64 && ctxt.Framepointer_enabled && autoffset > 0 && p.From3.Offset&obj.NOFRAME == 0 {
                // Make room for to save a base pointer. If autoffset == 0,
                // this might do something special like a tail jump to
                // another function, so in that case we omit this.

diff --git a/src/cmd/link/internal/ld/dwarf.go b/src/cmd/link/internal/ld/dwarf.go
index ca86e72d8329ccfcab45fe255f4901eec949b899..01747c543017a742608a42f6c1a938614fdac801 100644 (file)
--- a/src/cmd/link/internal/ld/dwarf.go
+++ b/src/cmd/link/internal/ld/dwarf.go
@@ -1558,7 +1558,7 @@ func writelines(prev *LSym) *LSym {
                                if !haslinkregister() {
                                        offs -= int64(SysArch.PtrSize)
                                }
-                               if obj.Framepointer_enabled != 0 {
+                               if obj.Framepointer_enabled(obj.Getgoos(), obj.Getgoarch()) {
                                        // The frame pointer is saved
                                        // between the CFA and the
                                        // autos.

diff --git a/src/cmd/link/internal/ld/lib.go b/src/cmd/link/internal/ld/lib.go
index da00de8547c748f653525fda3d94d38fe9ca70b3..bab71fb311437633bbfcb921f0713ae9cef5649e 100644 (file)
--- a/src/cmd/link/internal/ld/lib.go
+++ b/src/cmd/link/internal/ld/lib.go
@@ -639,11 +639,17 @@ func loadlib() {
                // recording the value of GOARM.
                if SysArch.Family == sys.ARM {
                        s := Linklookup(Ctxt, "runtime.goarm", 0)
-
                        s.Type = obj.SRODATA
                        s.Size = 0
                        Adduint8(Ctxt, s, uint8(Ctxt.Goarm))
                }
+
+               if obj.Framepointer_enabled(obj.Getgoos(), obj.Getgoarch()) {
+                       s := Linklookup(Ctxt, "runtime.framepointer_enabled", 0)
+                       s.Type = obj.SRODATA
+                       s.Size = 0
+                       Adduint8(Ctxt, s, 1)
+               }
        } else {
                // If OTOH the module does not contain the runtime package,
                // create a local symbol for the moduledata.

diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index ee895471046e62decc2c79b4872433d69a775ad6..727c991a577c1c955631bde6e41ef613e9e5465f 100644 (file)
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -434,9 +434,6 @@ func schedinit() {
 
        sched.maxmcount = 10000
 
-       // Cache the framepointer experiment. This affects stack unwinding.
-       framepointer_enabled = haveexperiment("framepointer")
-
        tracebackinit()
        moduledataverify()
        stackinit()
@@ -4163,6 +4160,9 @@ func setMaxThreads(in int) (out int) {
 }
 
 func haveexperiment(name string) bool {
+       if name == "framepointer" {
+               return framepointer_enabled // set by linker
+       }
        x := sys.Goexperiment
        for x != "" {
                xname := ""
@@ -4175,6 +4175,9 @@ func haveexperiment(name string) bool {
                if xname == name {
                        return true
                }
+               if len(xname) > 2 && xname[:2] == "no" && xname[2:] == name {
+                       return false
+               }
        }
        return false
 }

diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go
index 71da504f1c0e193cabf0c2ddf6e7f0dfae80197e..6119e75203c457c2a985ab9a1a22f1b145267ec2 100644 (file)
--- a/src/runtime/runtime2.go
+++ b/src/runtime/runtime2.go
@@ -725,7 +725,8 @@ var (
        support_avx       bool
        support_avx2      bool
 
-       goarm uint8 // set by cmd/link on arm systems
+       goarm                uint8 // set by cmd/link on arm systems
+       framepointer_enabled bool  // set by cmd/link
 )
 
 // Set by the linker so the runtime can determine the buildmode.

diff --git a/src/runtime/stack.go b/src/runtime/stack.go
index 33d29f19a8cedcb7759676da9df392b14dae406b..8e344cdf03585c3d07e9c9ee95b3d7dfcb28c744 100644 (file)
--- a/src/runtime/stack.go
+++ b/src/runtime/stack.go
@@ -155,9 +155,6 @@ var stackLarge struct {
        free [_MHeapMap_Bits]mSpanList // free lists by log_2(s.npages)
 }
 
-// Cached value of haveexperiment("framepointer")
-var framepointer_enabled bool
-
 func stackinit() {
        if _StackCacheSize&_PageMask != 0 {
                throw("cache size must be a multiple of page size")