Skip to content

Commit

Permalink
cmd/compile: move Used from gc.Node to gc.Name
Browse files Browse the repository at this point in the history
Node.Used was written to from the backend
concurrently with reads of Node.Class
for the same ONAME Nodes.
I do not know why it was not failing consistently
under the race detector, but it is a race.

This is likely also a problem with Node.HasVal and Node.HasOpt.
They will be handled in a separate CL.

Fix Used by moving it to gc.Name and making it a separate bool.
There was one non-Name use of Used, marking OLABELs as used.
That is no longer needed, now that goto and label checking
happens early in the front end.

Leave the getters and setters in place,
to ease changing the representation in the future
(or changing to an interface!).

Updates #20144

Change-Id: I9bec7c6d33dcb129a4cfa9d338462ea33087f9f7
Reviewed-on: https://go-review.googlesource.com/42015
Run-TryBot: Josh Bleecher Snyder <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>
  • Loading branch information
josharian committed Apr 27, 2017
1 parent 94d540a commit fc08a19
Show file tree
Hide file tree
Showing 15 changed files with 85 additions and 75 deletions.
4 changes: 2 additions & 2 deletions src/cmd/compile/internal/gc/closure.go
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ func transformclosure(xfunc *Node) {
addr := newname(lookup("&" + v.Sym.Name))
addr.Type = types.NewPtr(v.Type)
addr.SetClass(PAUTO)
addr.SetUsed(true)
addr.Name.SetUsed(true)
addr.Name.Curfn = xfunc
xfunc.Func.Dcl = append(xfunc.Func.Dcl, addr)
v.Name.Param.Heapaddr = addr
Expand Down Expand Up @@ -622,7 +622,7 @@ func makepartialcall(fn *Node, t0 *types.Type, meth *types.Sym) *Node {
}
ptr := newname(lookup("rcvr"))
ptr.SetClass(PAUTO)
ptr.SetUsed(true)
ptr.Name.SetUsed(true)
ptr.Name.Curfn = xfunc
xfunc.Func.Dcl = append(xfunc.Func.Dcl, ptr)
var body []*Node
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/gc/esc.go
Original file line number Diff line number Diff line change
Expand Up @@ -1443,7 +1443,7 @@ func (e *EscState) initEscRetval(call *Node, fntype *types.Type) {
ret.SetClass(PAUTO)
ret.Name.Curfn = Curfn
e.nodeEscState(ret).Loopdepth = e.loopdepth
ret.SetUsed(true)
ret.Name.SetUsed(true)
ret.Pos = call.Pos
cE.Retval.Append(ret)
}
Expand Down
6 changes: 3 additions & 3 deletions src/cmd/compile/internal/gc/fmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -520,11 +520,11 @@ func (n *Node) jconv(s fmt.State, flag FmtFlag) {
}

if c == 0 && n.HasCall() {
fmt.Fprintf(s, " hascall")
fmt.Fprint(s, " hascall")
}

if c == 0 && n.Used() {
fmt.Fprintf(s, " used(%v)", n.Used())
if c == 0 && n.Name != nil && n.Name.Used() {
fmt.Fprint(s, " used")
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/cmd/compile/internal/gc/gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,13 @@ func tempnamel(pos src.XPos, curfn *Node, nn *Node, t *types.Type) {
func temp(t *types.Type) *Node {
var n Node
tempnamel(lineno, Curfn, &n, t)
asNode(n.Sym.Def).SetUsed(true)
asNode(n.Sym.Def).Name.SetUsed(true)
return n.Orig
}

func tempAt(pos src.XPos, curfn *Node, t *types.Type) *Node {
var n Node
tempnamel(pos, curfn, &n, t)
asNode(n.Sym.Def).SetUsed(true)
asNode(n.Sym.Def).Name.SetUsed(true)
return n.Orig
}
7 changes: 3 additions & 4 deletions src/cmd/compile/internal/gc/inl.go
Original file line number Diff line number Diff line change
Expand Up @@ -745,7 +745,6 @@ func mkinlcall1(n *Node, fn *Node, isddd bool) *Node {
body := subst.list(fn.Func.Inl)

lab := nod(OLABEL, retlabel, nil)
lab.SetUsed(true) // avoid 'not used' when function doesn't have return
body = append(body, lab)

typecheckslice(body, Etop)
Expand Down Expand Up @@ -817,7 +816,7 @@ func inlvar(var_ *Node) *Node {
n := newname(var_.Sym)
n.Type = var_.Type
n.SetClass(PAUTO)
n.SetUsed(true)
n.Name.SetUsed(true)
n.Name.Curfn = Curfn // the calling function, not the called one
n.SetAddrtaken(var_.Addrtaken())

Expand All @@ -830,7 +829,7 @@ func retvar(t *types.Field, i int) *Node {
n := newname(lookupN("~r", i))
n.Type = t.Type
n.SetClass(PAUTO)
n.SetUsed(true)
n.Name.SetUsed(true)
n.Name.Curfn = Curfn // the calling function, not the called one
Curfn.Func.Dcl = append(Curfn.Func.Dcl, n)
return n
Expand All @@ -842,7 +841,7 @@ func argvar(t *types.Type, i int) *Node {
n := newname(lookupN("~arg", i))
n.Type = t.Elem()
n.SetClass(PAUTO)
n.SetUsed(true)
n.Name.SetUsed(true)
n.Name.Curfn = Curfn // the calling function, not the called one
Curfn.Func.Dcl = append(Curfn.Func.Dcl, n)
return n
Expand Down
6 changes: 3 additions & 3 deletions src/cmd/compile/internal/gc/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -1046,7 +1046,7 @@ func clearImports() {
// leave s->block set to cause redeclaration
// errors if a conflicting top-level name is
// introduced by a different file.
if !asNode(s.Def).Used() && nsyntaxerrors == 0 {
if !asNode(s.Def).Name.Used() && nsyntaxerrors == 0 {
pkgnotused(asNode(s.Def).Pos, asNode(s.Def).Name.Pkg.Path, s.Name)
}
s.Def = nil
Expand All @@ -1056,9 +1056,9 @@ func clearImports() {
if IsAlias(s) {
// throw away top-level name left over
// from previous import . "x"
if asNode(s.Def).Name != nil && asNode(s.Def).Name.Pack != nil && !asNode(s.Def).Name.Pack.Used() && nsyntaxerrors == 0 {
if asNode(s.Def).Name != nil && asNode(s.Def).Name.Pack != nil && !asNode(s.Def).Name.Pack.Name.Used() && nsyntaxerrors == 0 {
pkgnotused(asNode(s.Def).Name.Pack.Pos, asNode(s.Def).Name.Pack.Name.Pkg.Path, "")
asNode(s.Def).Name.Pack.SetUsed(true)
asNode(s.Def).Name.Pack.Name.SetUsed(true)
}

s.Def = nil
Expand Down
8 changes: 4 additions & 4 deletions src/cmd/compile/internal/gc/noder.go
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ func (p *noder) expr(expr syntax.Expr) *Node {
// parser.new_dotname
obj := p.expr(expr.X)
if obj.Op == OPACK {
obj.SetUsed(true)
obj.Name.SetUsed(true)
return oldname(restrictlookup(expr.Sel.Value, obj.Name.Pkg))
}
return p.setlineno(expr, nodSym(OXDOT, obj, p.name(expr.Sel)))
Expand Down Expand Up @@ -611,7 +611,7 @@ func (p *noder) packname(expr syntax.Expr) *types.Sym {
case *syntax.Name:
name := p.name(expr)
if n := oldname(name); n.Name != nil && n.Name.Pack != nil {
n.Name.Pack.SetUsed(true)
n.Name.Pack.Name.SetUsed(true)
}
return name
case *syntax.SelectorExpr:
Expand All @@ -621,7 +621,7 @@ func (p *noder) packname(expr syntax.Expr) *types.Sym {
yyerror("%v is not a package", name)
pkg = localpkg
} else {
asNode(name.Def).SetUsed(true)
asNode(name.Def).Name.SetUsed(true)
pkg = asNode(name.Def).Name.Pkg
}
return restrictlookup(expr.Sel.Value, pkg)
Expand Down Expand Up @@ -1125,7 +1125,7 @@ func (p *noder) pragma(pos src.Pos, text string) syntax.Pragma {
func mkname(sym *types.Sym) *Node {
n := oldname(sym)
if n.Name != nil && n.Name.Pack != nil {
n.Name.Pack.SetUsed(true)
n.Name.Pack.Name.SetUsed(true)
}
return n
}
Expand Down
16 changes: 8 additions & 8 deletions src/cmd/compile/internal/gc/pgen.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ func cmpstackvarlt(a, b *Node) bool {
return a.Xoffset < b.Xoffset
}

if a.Used() != b.Used() {
return a.Used()
if a.Name.Used() != b.Name.Used() {
return a.Name.Used()
}

ap := types.Haspointers(a.Type)
Expand Down Expand Up @@ -118,13 +118,13 @@ func (s *ssafn) AllocFrame(f *ssa.Func) {
// Mark the PAUTO's unused.
for _, ln := range fn.Dcl {
if ln.Class() == PAUTO {
ln.SetUsed(false)
ln.Name.SetUsed(false)
}
}

for _, l := range f.RegAlloc {
if ls, ok := l.(ssa.LocalSlot); ok {
ls.N.(*Node).SetUsed(true)
ls.N.(*Node).Name.SetUsed(true)
}
}

Expand All @@ -136,10 +136,10 @@ func (s *ssafn) AllocFrame(f *ssa.Func) {
n := a.Node.(*Node)
// Don't modify nodfp; it is a global.
if n != nodfp {
n.SetUsed(true)
n.Name.SetUsed(true)
}
case *ssa.AutoSymbol:
a.Node.(*Node).SetUsed(true)
a.Node.(*Node).Name.SetUsed(true)
}

if !scratchUsed {
Expand All @@ -159,7 +159,7 @@ func (s *ssafn) AllocFrame(f *ssa.Func) {
if n.Op != ONAME || n.Class() != PAUTO {
continue
}
if !n.Used() {
if !n.Name.Used() {
fn.Dcl = fn.Dcl[:i]
break
}
Expand Down Expand Up @@ -308,7 +308,7 @@ func debuginfo(fnsym *obj.LSym, curfn interface{}) []*dwarf.Var {

switch n.Class() {
case PAUTO:
if !n.Used() {
if !n.Name.Used() {
Fatalf("debuginfo unused node (AllocFrame should truncate fn.Func.Dcl)")
}
name = obj.NAME_AUTO
Expand Down
83 changes: 47 additions & 36 deletions src/cmd/compile/internal/gc/pgen_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,19 @@ func typeWithPointers() *types.Type {
return t
}

func markUsed(n *Node) *Node {
n.Name.SetUsed(true)
return n
}

func markNeedZero(n *Node) *Node {
n.Name.SetNeedzero(true)
return n
}

func nodeWithClass(n Node, c Class) *Node {
n.SetClass(c)
n.Name = new(Name)
return &n
}

Expand Down Expand Up @@ -72,13 +83,13 @@ func TestCmpstackvar(t *testing.T) {
true,
},
{
nodeWithClass(Node{flags: nodeUsed}, PAUTO),
markUsed(nodeWithClass(Node{}, PAUTO)),
nodeWithClass(Node{}, PAUTO),
true,
},
{
nodeWithClass(Node{}, PAUTO),
nodeWithClass(Node{flags: nodeUsed}, PAUTO),
markUsed(nodeWithClass(Node{}, PAUTO)),
false,
},
{
Expand All @@ -92,13 +103,13 @@ func TestCmpstackvar(t *testing.T) {
true,
},
{
nodeWithClass(Node{Type: &types.Type{}, Name: &Name{flags: nameNeedzero}}, PAUTO),
markNeedZero(nodeWithClass(Node{Type: &types.Type{}}, PAUTO)),
nodeWithClass(Node{Type: &types.Type{}, Name: &Name{}}, PAUTO),
true,
},
{
nodeWithClass(Node{Type: &types.Type{}, Name: &Name{}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Name: &Name{flags: nameNeedzero}}, PAUTO),
markNeedZero(nodeWithClass(Node{Type: &types.Type{}}, PAUTO)),
false,
},
{
Expand All @@ -112,18 +123,18 @@ func TestCmpstackvar(t *testing.T) {
true,
},
{
nodeWithClass(Node{Type: &types.Type{}, Name: &Name{}, Sym: &types.Sym{Name: "abc"}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Name: &Name{}, Sym: &types.Sym{Name: "xyz"}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Sym: &types.Sym{Name: "abc"}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Sym: &types.Sym{Name: "xyz"}}, PAUTO),
true,
},
{
nodeWithClass(Node{Type: &types.Type{}, Name: &Name{}, Sym: &types.Sym{Name: "abc"}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Name: &Name{}, Sym: &types.Sym{Name: "abc"}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Sym: &types.Sym{Name: "abc"}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Sym: &types.Sym{Name: "abc"}}, PAUTO),
false,
},
{
nodeWithClass(Node{Type: &types.Type{}, Name: &Name{}, Sym: &types.Sym{Name: "xyz"}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Name: &Name{}, Sym: &types.Sym{Name: "abc"}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Sym: &types.Sym{Name: "xyz"}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Sym: &types.Sym{Name: "abc"}}, PAUTO),
false,
},
}
Expand All @@ -141,34 +152,34 @@ func TestCmpstackvar(t *testing.T) {

func TestStackvarSort(t *testing.T) {
inp := []*Node{
nodeWithClass(Node{Type: &types.Type{}, Name: &Name{}, Sym: &types.Sym{}}, PFUNC),
nodeWithClass(Node{Type: &types.Type{}, Name: &Name{}, Sym: &types.Sym{}}, PAUTO),
nodeWithClass(Node{Xoffset: 0, Type: &types.Type{}, Name: &Name{}, Sym: &types.Sym{}}, PFUNC),
nodeWithClass(Node{Xoffset: 10, Type: &types.Type{}, Name: &Name{}, Sym: &types.Sym{}}, PFUNC),
nodeWithClass(Node{Xoffset: 20, Type: &types.Type{}, Name: &Name{}, Sym: &types.Sym{}}, PFUNC),
nodeWithClass(Node{flags: nodeUsed, Type: &types.Type{}, Name: &Name{}, Sym: &types.Sym{}}, PAUTO),
nodeWithClass(Node{Type: typeWithoutPointers(), Name: &Name{}, Sym: &types.Sym{}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Name: &Name{}, Sym: &types.Sym{}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Name: &Name{flags: nameNeedzero}, Sym: &types.Sym{}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{Width: 1}, Name: &Name{}, Sym: &types.Sym{}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{Width: 2}, Name: &Name{}, Sym: &types.Sym{}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Name: &Name{}, Sym: &types.Sym{Name: "abc"}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Name: &Name{}, Sym: &types.Sym{Name: "xyz"}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Sym: &types.Sym{}}, PFUNC),
nodeWithClass(Node{Type: &types.Type{}, Sym: &types.Sym{}}, PAUTO),
nodeWithClass(Node{Xoffset: 0, Type: &types.Type{}, Sym: &types.Sym{}}, PFUNC),
nodeWithClass(Node{Xoffset: 10, Type: &types.Type{}, Sym: &types.Sym{}}, PFUNC),
nodeWithClass(Node{Xoffset: 20, Type: &types.Type{}, Sym: &types.Sym{}}, PFUNC),
markUsed(nodeWithClass(Node{Type: &types.Type{}, Sym: &types.Sym{}}, PAUTO)),
nodeWithClass(Node{Type: typeWithoutPointers(), Sym: &types.Sym{}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Sym: &types.Sym{}}, PAUTO),
markNeedZero(nodeWithClass(Node{Type: &types.Type{}, Sym: &types.Sym{}}, PAUTO)),
nodeWithClass(Node{Type: &types.Type{Width: 1}, Sym: &types.Sym{}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{Width: 2}, Sym: &types.Sym{}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Sym: &types.Sym{Name: "abc"}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Sym: &types.Sym{Name: "xyz"}}, PAUTO),
}
want := []*Node{
nodeWithClass(Node{Type: &types.Type{}, Name: &Name{}, Sym: &types.Sym{}}, PFUNC),
nodeWithClass(Node{Xoffset: 0, Type: &types.Type{}, Name: &Name{}, Sym: &types.Sym{}}, PFUNC),
nodeWithClass(Node{Xoffset: 10, Type: &types.Type{}, Name: &Name{}, Sym: &types.Sym{}}, PFUNC),
nodeWithClass(Node{Xoffset: 20, Type: &types.Type{}, Name: &Name{}, Sym: &types.Sym{}}, PFUNC),
nodeWithClass(Node{flags: nodeUsed, Type: &types.Type{}, Name: &Name{}, Sym: &types.Sym{}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Name: &Name{flags: nameNeedzero}, Sym: &types.Sym{}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{Width: 2}, Name: &Name{}, Sym: &types.Sym{}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{Width: 1}, Name: &Name{}, Sym: &types.Sym{}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Name: &Name{}, Sym: &types.Sym{}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Name: &Name{}, Sym: &types.Sym{}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Name: &Name{}, Sym: &types.Sym{Name: "abc"}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Name: &Name{}, Sym: &types.Sym{Name: "xyz"}}, PAUTO),
nodeWithClass(Node{Type: typeWithoutPointers(), Name: &Name{}, Sym: &types.Sym{}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Sym: &types.Sym{}}, PFUNC),
nodeWithClass(Node{Xoffset: 0, Type: &types.Type{}, Sym: &types.Sym{}}, PFUNC),
nodeWithClass(Node{Xoffset: 10, Type: &types.Type{}, Sym: &types.Sym{}}, PFUNC),
nodeWithClass(Node{Xoffset: 20, Type: &types.Type{}, Sym: &types.Sym{}}, PFUNC),
markUsed(nodeWithClass(Node{Type: &types.Type{}, Sym: &types.Sym{}}, PAUTO)),
markNeedZero(nodeWithClass(Node{Type: &types.Type{}, Sym: &types.Sym{}}, PAUTO)),
nodeWithClass(Node{Type: &types.Type{Width: 2}, Sym: &types.Sym{}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{Width: 1}, Sym: &types.Sym{}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Sym: &types.Sym{}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Sym: &types.Sym{}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Sym: &types.Sym{Name: "abc"}}, PAUTO),
nodeWithClass(Node{Type: &types.Type{}, Sym: &types.Sym{Name: "xyz"}}, PAUTO),
nodeWithClass(Node{Type: typeWithoutPointers(), Sym: &types.Sym{}}, PAUTO),
}
// haspointers updates Type.Haspointers as a side effect, so
// exercise this function on all inputs so that reflect.DeepEqual
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/gc/plive.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ func (lv *Liveness) valueEffects(v *ssa.Value) (pos int32, effect liveEffect) {
// variable" ICEs (issue 19632).
switch v.Op {
case ssa.OpVarDef, ssa.OpVarKill, ssa.OpVarLive, ssa.OpKeepAlive:
if !n.Used() {
if !n.Name.Used() {
return -1, 0
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/gc/ssa.go
Original file line number Diff line number Diff line change
Expand Up @@ -4964,7 +4964,7 @@ func (e *ssafn) namedAuto(name string, typ ssa.Type, pos src.XPos) ssa.GCNode {
n.Orig = n

s.Def = asTypesNode(n)
asNode(s.Def).SetUsed(true)
asNode(s.Def).Name.SetUsed(true)
n.Sym = s
n.Type = t
n.SetClass(PAUTO)
Expand Down
Loading

0 comments on commit fc08a19

Please sign in to comment.