Skip to content

Commit

Permalink
internal/pkgbits: change EnableSync into a dynamic knob
Browse files Browse the repository at this point in the history
Rather than requiring users to recompile the compiler and all tools to
enable/disable sync markers, this CL adds a flag word into the Unified
IR file format to allow indicating whether they're enabled or not.
This in turn requires bumping the file format version.

Thanks to drchase@ for benchmarks showing this isn't as expensive as I
feared it would be.

Change-Id: I99afa0ee0b6ef5f30ed8ca840805ff9fd46b1857
Reviewed-on: https://go-review.googlesource.com/c/go/+/417097
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
  • Loading branch information
mdempsky committed Jul 12, 2022
1 parent d667be8 commit 9371a65
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 20 deletions.
1 change: 1 addition & 0 deletions src/cmd/compile/internal/base/flag.go
Expand Up @@ -165,6 +165,7 @@ func ParseFlags() {
if buildcfg.Experiment.Unified {
Debug.Unified = 1
}
Debug.SyncFrames = -1 // disable sync markers by default

Debug.Checkptr = -1 // so we can tell whether it is set explicitly

Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/noder/reader.go
Expand Up @@ -1072,7 +1072,7 @@ func (r *reader) addLocal(name *ir.Name, ctxt ir.Class) {
assert(ctxt == ir.PAUTO || ctxt == ir.PPARAM || ctxt == ir.PPARAMOUT)

r.Sync(pkgbits.SyncAddLocal)
if pkgbits.EnableSync {
if r.p.SyncMarkers() {
want := r.Int()
if have := len(r.locals); have != want {
base.FatalfAt(name.Pos(), "locals table has desynced")
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/noder/writer.go
Expand Up @@ -995,7 +995,7 @@ func (w *writer) funcarg(param *types2.Var, result bool) {
func (w *writer) addLocal(obj *types2.Var) {
w.Sync(pkgbits.SyncAddLocal)
idx := len(w.localsIdx)
if pkgbits.EnableSync {
if w.p.SyncMarkers() {
w.Int(idx)
}
if w.localsIdx == nil {
Expand Down
26 changes: 22 additions & 4 deletions src/internal/pkgbits/decoder.go
Expand Up @@ -18,6 +18,12 @@ import (
// A PkgDecoder provides methods for decoding a package's Unified IR
// export data.
type PkgDecoder struct {
// version is the file format version.
version uint32

// sync indicates whether the file uses sync markers.
sync bool

// pkgPath is the package path for the package to be decoded.
//
// TODO(mdempsky): Remove; unneeded since CL 391014.
Expand Down Expand Up @@ -52,6 +58,9 @@ type PkgDecoder struct {
// TODO(mdempsky): Remove; unneeded since CL 391014.
func (pr *PkgDecoder) PkgPath() string { return pr.pkgPath }

// SyncMarkers reports whether pr uses sync markers.
func (pr *PkgDecoder) SyncMarkers() bool { return pr.sync }

// NewPkgDecoder returns a PkgDecoder initialized to read the Unified
// IR export data from input. pkgPath is the package path for the
// compilation unit that produced the export data.
Expand All @@ -67,9 +76,18 @@ func NewPkgDecoder(pkgPath, input string) PkgDecoder {

r := strings.NewReader(input)

var version uint32
assert(binary.Read(r, binary.LittleEndian, &version) == nil)
assert(version == 0)
assert(binary.Read(r, binary.LittleEndian, &pr.version) == nil)

switch pr.version {
default:
panic(fmt.Errorf("unsupported version: %v", pr.version))
case 0:
// no flags
case 1:
var flags uint32
assert(binary.Read(r, binary.LittleEndian, &flags) == nil)
pr.sync = flags&flagSyncMarkers != 0
}

assert(binary.Read(r, binary.LittleEndian, pr.elemEndsEnds[:]) == nil)

Expand Down Expand Up @@ -215,7 +233,7 @@ func (r *Decoder) rawReloc(k RelocKind, idx int) Index {
//
// If EnableSync is false, then Sync is a no-op.
func (r *Decoder) Sync(mWant SyncMarker) {
if !EnableSync {
if !r.common.sync {
return
}

Expand Down
25 changes: 22 additions & 3 deletions src/internal/pkgbits/encoder.go
Expand Up @@ -14,6 +14,13 @@ import (
"runtime"
)

// currentVersion is the current version number.
//
// - v0: initial prototype
//
// - v1: adds the flags uint32 word
const currentVersion uint32 = 1

// A PkgEncoder provides methods for encoding a package's Unified IR
// export data.
type PkgEncoder struct {
Expand All @@ -25,15 +32,21 @@ type PkgEncoder struct {
// elems[RelocString][stringsIdx[s]] == s (if present).
stringsIdx map[string]Index

// syncFrames is the number of frames to write at each sync
// marker. A negative value means sync markers are omitted.
syncFrames int
}

// SyncMarkers reports whether pw uses sync markers.
func (pw *PkgEncoder) SyncMarkers() bool { return pw.syncFrames >= 0 }

// NewPkgEncoder returns an initialized PkgEncoder.
//
// syncFrames is the number of caller frames that should be serialized
// at Sync points. Serializing additional frames results in larger
// export data files, but can help diagnosing desync errors in
// higher-level Unified IR reader/writer code.
// higher-level Unified IR reader/writer code. If syncFrames is
// negative, then sync markers are omitted entirely.
func NewPkgEncoder(syncFrames int) PkgEncoder {
return PkgEncoder{
stringsIdx: make(map[string]Index),
Expand All @@ -51,7 +64,13 @@ func (pw *PkgEncoder) DumpTo(out0 io.Writer) (fingerprint [8]byte) {
assert(binary.Write(out, binary.LittleEndian, x) == nil)
}

writeUint32(0) // version
writeUint32(currentVersion)

var flags uint32
if pw.SyncMarkers() {
flags |= flagSyncMarkers
}
writeUint32(flags)

// Write elemEndsEnds.
var sum uint32
Expand Down Expand Up @@ -204,7 +223,7 @@ func (w *Encoder) rawReloc(r RelocKind, idx Index) int {
}

func (w *Encoder) Sync(m SyncMarker) {
if !EnableSync {
if !w.p.SyncMarkers() {
return
}

Expand Down
9 changes: 9 additions & 0 deletions src/internal/pkgbits/flags.go
@@ -0,0 +1,9 @@
// Copyright 2022 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 pkgbits

const (
flagSyncMarkers = 1 << iota // file format contains sync markers
)
11 changes: 0 additions & 11 deletions src/internal/pkgbits/sync.go
Expand Up @@ -9,17 +9,6 @@ import (
"strings"
)

// EnableSync controls whether sync markers are written into unified
// IR's export data format and also whether they're expected when
// reading them back in. They're inessential to the correct
// functioning of unified IR, but are helpful during development to
// detect mistakes.
//
// When sync is enabled, writer stack frames will also be included in
// the export data. Currently, a fixed number of frames are included,
// controlled by -d=syncframes (default 0).
const EnableSync = true

// fmtFrames formats a backtrace for reporting reader/writer desyncs.
func fmtFrames(pcs ...uintptr) []string {
res := make([]string, 0, len(pcs))
Expand Down

0 comments on commit 9371a65

Please sign in to comment.