From 736390c2bd2d7f00d62ca62f18836f82eb1f51a3 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Wed, 25 Apr 2018 12:36:36 -0700 Subject: [PATCH] cmd/compile: allow SSA of multi-field structs while instrumenting When we moved racewalk to buildssa, we disabled SSA of structs with 2-4 fields while instrumenting because it caused false dependencies because for "x.f" we would emit (StructSelect (Load (Addr x)) "f") Even though we later simplify this to (Load (OffPtr (Addr x) "f")) the instrumentation saw a load of x in its entirety and would issue appropriate race/msan calls. The fix taken here is to directly emit the OffPtr form when x.f is addressable and can't be represented in SSA form. Change-Id: I0caf37bced52e9c16937466b0ac8cab6d356e525 Reviewed-on: https://go-review.googlesource.com/109360 Run-TryBot: Matthew Dempsky TryBot-Result: Gobot Gobot Reviewed-by: Cherry Zhang --- src/cmd/compile/internal/gc/ssa.go | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/cmd/compile/internal/gc/ssa.go b/src/cmd/compile/internal/gc/ssa.go index b286470e2d..a3d2230964 100644 --- a/src/cmd/compile/internal/gc/ssa.go +++ b/src/cmd/compile/internal/gc/ssa.go @@ -2108,11 +2108,6 @@ func (s *state) expr(n *Node) *ssa.Value { return s.load(n.Type, p) case ODOT: - t := n.Left.Type - if canSSAType(t) { - v := s.expr(n.Left) - return s.newValue1I(ssa.OpStructSelect, n.Type, int64(fieldIdx(n)), v) - } if n.Left.Op == OSTRUCTLIT { // All literals with nonzero fields have already been // rewritten during walk. Any that remain are just T{} @@ -2122,8 +2117,16 @@ func (s *state) expr(n *Node) *ssa.Value { } return s.zeroVal(n.Type) } - p := s.addr(n, false) - return s.load(n.Type, p) + // If n is addressable and can't be represented in + // SSA, then load just the selected field. This + // prevents false memory dependencies in race/msan + // instrumentation. + if islvalue(n) && !s.canSSA(n) { + p := s.addr(n, false) + return s.load(n.Type, p) + } + v := s.expr(n.Left) + return s.newValue1I(ssa.OpStructSelect, n.Type, int64(fieldIdx(n)), v) case ODOTPTR: p := s.exprPtr(n.Left, false, n.Pos) @@ -3735,14 +3738,6 @@ func canSSAType(t *types.Type) bool { } return false case TSTRUCT: - // When instrumenting, don't SSA structs with more - // than one field. Otherwise, an access like "x.f" may - // be compiled into a full load of x, which can - // introduce false dependencies on other "x.g" fields. - if instrumenting && t.NumFields() > 1 { - return false - } - if t.NumFields() > ssa.MaxStruct { return false } -- 2.48.1