]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: allow SSA of multi-field structs while instrumenting
authorMatthew Dempsky <mdempsky@google.com>
Wed, 25 Apr 2018 19:36:36 +0000 (12:36 -0700)
committerMatthew Dempsky <mdempsky@google.com>
Thu, 26 Apr 2018 07:07:18 +0000 (07:07 +0000)
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 <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
src/cmd/compile/internal/gc/ssa.go

index b286470e2d13bf0f8590d8ba40c93972f880c9fd..a3d2230964ba5d373098fe820a6df47e57a9b9a1 100644 (file)
@@ -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
                }