internal/pkgbits: change EnableSync into a dynamic knob

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>

src/cmd/compile/internal/base/flag.go

@@ -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
 

src/cmd/compile/internal/noder/reader.go

@@ -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")

src/cmd/compile/internal/noder/writer.go

@@ -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 {

src/internal/pkgbits/decoder.go

@@ -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.
@@ -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.
@@ -67,9 +76,18 @@ func NewPkgDecoder(pkgPath, input string) PkgDecoder {
 
 	r := strings.NewReader(input)
 
-	var version uint32
+	assert(binary.Read(r, binary.LittleEndian, &pr.version) == nil)
-	assert(binary.Read(r, binary.LittleEndian, &version) == nil)
+
-	assert(version == 0)
+	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)
 
@@ -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
 	}
 

src/internal/pkgbits/encoder.go

@@ -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 {
@@ -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),
@@ -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
@@ -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
 	}
 

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
+)

src/internal/pkgbits/sync.go

@@ -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))