Go patch committed: Call gcWrwiteBarrier instead of writebarrierptr

Message ID CAOyqgcXkTrVqJEFbu_BoC+uHZqZobyhp-BAat9d6Qt7Beo2PGA@mail.gmail.com
State New
Headers show
Series
  • Go patch committed: Call gcWrwiteBarrier instead of writebarrierptr
Related show

Commit Message

Ian Lance Taylor Sept. 13, 2018, 10:26 p.m.
In the Go 1.11 release writebarrierptr is going away, so change the Go
frontend to call gcWriteBarrier instead.  We weren't using
gcWriteBarrier before; adjust the implementation to use the putFast
method.

This revealed a problem in the kickoff function.  When using cgo,
kickoff can be called on the g0 of an m allocated by newExtraM.  In
that case the m will generally have a p, but systemstack may be called
by wbBufFlush as part of flushing the write barrier buffer.  At that
point the buffer is full, so we can not do a write barrier.  So adjust
the existing code in kickoff so that in the case where we are g0,
don't do any write barrier at all.

Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

Ian

Patch

Index: gcc/go/gofrontend/MERGE
===================================================================
--- gcc/go/gofrontend/MERGE	(revision 264294)
+++ gcc/go/gofrontend/MERGE	(working copy)
@@ -1,4 +1,4 @@ 
-82d7205ba9e5c1fe38fd24f89a45caf2e974975b
+218c9159635e06e39ae43d0efe1ac1e694fead2e
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/runtime.def
===================================================================
--- gcc/go/gofrontend/runtime.def	(revision 264290)
+++ gcc/go/gofrontend/runtime.def	(working copy)
@@ -302,7 +302,7 @@  DEF_GO_RUNTIME(IFACEEFACEEQ, "runtime.if
 
 
 // Set *dst = src where dst is a pointer to a pointer and src is a pointer.
-DEF_GO_RUNTIME(WRITEBARRIERPTR, "runtime.writebarrierptr",
+DEF_GO_RUNTIME(GCWRITEBARRIER, "runtime.gcWriteBarrier",
 	       P2(POINTER, POINTER), R0())
 
 // Set *dst = *src for an arbitrary type.
Index: gcc/go/gofrontend/wb.cc
===================================================================
--- gcc/go/gofrontend/wb.cc	(revision 264283)
+++ gcc/go/gofrontend/wb.cc	(working copy)
@@ -380,7 +380,7 @@  Gogo::propagate_writebarrierrec()
 // This is compatible with the definition in the runtime package.
 //
 // For types that are pointer shared (pointers, maps, chans, funcs),
-// we replaced the call to typedmemmove with writebarrierptr(&A, B).
+// we replaced the call to typedmemmove with gcWriteBarrier(&A, B).
 // As far as the GC is concerned, all pointers are the same, so it
 // doesn't need the type descriptor.
 //
@@ -391,7 +391,7 @@  Gogo::propagate_writebarrierrec()
 // runtime package, so we could optimize by only testing it once
 // between function calls.
 //
-// A slice could be handled with a call to writebarrierptr plus two
+// A slice could be handled with a call to gcWriteBarrier plus two
 // integer moves.
 
 // Traverse the IR adding write barriers.
@@ -824,7 +824,7 @@  Gogo::assign_with_write_barrier(Function
     case Type::TYPE_MAP:
     case Type::TYPE_CHANNEL:
       // These types are all represented by a single pointer.
-      call = Runtime::make_call(Runtime::WRITEBARRIERPTR, loc, 2, lhs, rhs);
+      call = Runtime::make_call(Runtime::GCWRITEBARRIER, loc, 2, lhs, rhs);
       break;
 
     case Type::TYPE_STRING:
Index: libgo/go/runtime/mgc_gccgo.go
===================================================================
--- libgo/go/runtime/mgc_gccgo.go	(revision 264276)
+++ libgo/go/runtime/mgc_gccgo.go	(working copy)
@@ -11,6 +11,11 @@  import (
 	"unsafe"
 )
 
+// For gccgo, use go:linkname to rename compiler-called functions to
+// themselves, so that the compiler will export them.
+//
+//go:linkname gcWriteBarrier runtime.gcWriteBarrier
+
 // gcRoot is a single GC root: a variable plus a ptrmask.
 //go:notinheap
 type gcRoot struct {
@@ -188,12 +193,7 @@  func checkPreempt() {
 //go:nowritebarrier
 func gcWriteBarrier(dst *uintptr, src uintptr) {
 	buf := &getg().m.p.ptr().wbBuf
-	next := buf.next
-	np := next + 2*sys.PtrSize
-	buf.next = np
-	*(*uintptr)(unsafe.Pointer(next)) = src
-	*(*uintptr)(unsafe.Pointer(next + sys.PtrSize)) = *dst
-	if np >= buf.end {
+	if !buf.putFast(src, *dst) {
 		wbBufFlush(dst, src)
 	}
 	*dst = src
Index: libgo/go/runtime/proc.go
===================================================================
--- libgo/go/runtime/proc.go	(revision 264282)
+++ libgo/go/runtime/proc.go	(working copy)
@@ -1146,24 +1146,29 @@  func kickoff() {
 
 	fv := gp.entry
 	param := gp.param
-	gp.entry = nil
 
 	// When running on the g0 stack we can wind up here without a p,
-	// for example from mcall(exitsyscall0) in exitsyscall.
-	// Setting gp.param = nil will call a write barrier, and if
-	// there is no p that write barrier will crash. When called from
-	// mcall the gp.param value will be a *g, which we don't need to
-	// shade since we know it will be kept alive elsewhere. In that
-	// case clear the field using uintptr so that the write barrier
-	// does nothing.
-	if gp.m.p == 0 {
-		if gp == gp.m.g0 && gp.param == unsafe.Pointer(gp.m.curg) {
-			*(*uintptr)(unsafe.Pointer(&gp.param)) = 0
-		} else {
-			throw("no p in kickoff")
-		}
+	// for example from mcall(exitsyscall0) in exitsyscall, in
+	// which case we can not run a write barrier.
+	// It is also possible for us to get here from the systemstack
+	// call in wbBufFlush, at which point the write barrier buffer
+	// is full and we can not run a write barrier.
+	// Setting gp.entry = nil or gp.param = nil will try to run a
+	// write barrier, so if we are on the g0 stack due to mcall
+	// (systemstack calls mcall) then clear the field using uintptr.
+	// This is OK when gp.param is gp.m.curg, as curg will be kept
+	// alive elsewhere, and gp.entry always points into g, or
+	// to a statically allocated value, or (in the case of mcall)
+	// to the stack.
+	if gp == gp.m.g0 && gp.param == unsafe.Pointer(gp.m.curg) {
+		*(*uintptr)(unsafe.Pointer(&gp.entry)) = 0
+		*(*uintptr)(unsafe.Pointer(&gp.param)) = 0
+	} else if gp.m.p == 0 {
+		throw("no p in kickoff")
+	} else {
+		gp.entry = nil
+		gp.param = nil
 	}
-	gp.param = nil
 
 	fv(param)
 	goexit1()