Revert "cmd/compile: de-virtualize interface calls"

This reverts commit 4e0c7c3f61.

Reason for revert: The presence-of-optimization test program is fragile, breaks under noopt, and might break if the Go libraries are tweaked.  It needs to be (re)written without reference to other packages.

Change-Id: I3aaf1ab006a1a255f961a978e9c984341740e3c7
Reviewed-on: https://go-review.googlesource.com/38097
Reviewed-by: Keith Randall <khr@golang.org>

src/cmd/compile/internal/gc/main.go

@@ -483,11 +483,7 @@ func Main() {
 			}
 		}
 
-		// Just before compilation, compile itabs found on
-		// the right side of OCONVIFACE so that methods
-		// can be de-virtualized during compilation.
 		Curfn = nil
-		peekitabs()
 
 		// Phase 8: Compile top level functions.
 		// Don't use range--walk can add functions to xtop.

src/cmd/compile/internal/gc/reflect.go

@@ -16,15 +16,6 @@ import (
 type itabEntry struct {
 	t, itype *Type
 	sym      *Sym
-
-	// symbol of the itab itself;
-	// filled in lazily after typecheck
-	lsym *obj.LSym
-
-	// symbols of each method in
-	// the itab, sorted by byte offset;
-	// filled in at the same time as lsym
-	entries []*obj.LSym
 }
 
 type ptabEntry struct {
@@ -424,6 +415,7 @@ func imethods(t *Type) []*Sig {
 		// Generate the method body, so that compiled
 		// code can refer to it.
 		isym := methodsym(method, t, 0)
+
 		if !isym.Siggen() {
 			isym.SetSiggen(true)
 			genwrapper(t, f, isym, 0)
@@ -1387,78 +1379,6 @@ ok:
 	return s
 }
 
-// for each itabEntry, gather the methods on
-// the concrete type that implement the interface
-func peekitabs() {
-	for i := range itabs {
-		tab := &itabs[i]
-		methods := genfun(tab.t, tab.itype)
-		if len(methods) == 0 {
-			continue
-		}
-		tab.lsym = Linksym(tab.sym)
-		tab.entries = methods
-	}
-}
-
-// for the given concrete type and interface
-// type, return the (sorted) set of methods
-// on the concrete type that implement the interface
-func genfun(t, it *Type) []*obj.LSym {
-	if t == nil || it == nil {
-		return nil
-	}
-	sigs := imethods(it)
-	methods := methods(t)
-	out := make([]*obj.LSym, 0, len(sigs))
-	if len(sigs) == 0 {
-		return nil
-	}
-
-	// both sigs and methods are sorted by name,
-	// so we can find the intersect in a single pass
-	for _, m := range methods {
-		if m.name == sigs[0].name {
-			out = append(out, Linksym(m.isym))
-			sigs = sigs[1:]
-			if len(sigs) == 0 {
-				break
-			}
-		}
-	}
-
-	return out
-}
-
-// itabsym uses the information gathered in
-// peekitabs to de-virtualize interface methods.
-// Since this is called by the SSA backend, it shouldn't
-// generate additional Nodes, Syms, etc.
-func itabsym(it *obj.LSym, offset int64) *obj.LSym {
-	var syms []*obj.LSym
-	if it == nil {
-		return nil
-	}
-
-	for i := range itabs {
-		e := &itabs[i]
-		if e.lsym == it {
-			syms = e.entries
-			break
-		}
-	}
-	if syms == nil {
-		return nil
-	}
-
-	// keep this arithmetic in sync with *itab layout
-	methodnum := int((offset - 3*int64(Widthptr) - 8) / int64(Widthptr))
-	if methodnum >= len(syms) {
-		return nil
-	}
-	return syms[methodnum]
-}
-
 func dumptypestructs() {
 	// copy types from externdcl list to signatlist
 	for _, n := range externdcl {

src/cmd/compile/internal/gc/ssa.go

@@ -4967,10 +4967,6 @@ func (e *ssaExport) SplitArray(name ssa.LocalSlot) ssa.LocalSlot {
 	return ssa.LocalSlot{N: n, Type: et, Off: name.Off}
 }
 
-func (e *ssaExport) DerefItab(it *obj.LSym, offset int64) *obj.LSym {
-	return itabsym(it, offset)
-}
-
 // namedAuto returns a new AUTO variable with the given name and type.
 // These are exposed to the debugger.
 func (e *ssaExport) namedAuto(name string, typ ssa.Type) ssa.GCNode {

src/cmd/compile/internal/gc/subr.go

@@ -1684,6 +1684,7 @@ func structargs(tl *Type, mustname bool) []*Node {
 //	rcvr - U
 //	method - M func (t T)(), a TFIELD type struct
 //	newnam - the eventual mangled name of this function
+
 func genwrapper(rcvr *Type, method *Field, newnam *Sym, iface int) {
 	if false && Debug['r'] != 0 {
 		fmt.Printf("genwrapper rcvrtype=%v method=%v newnam=%v\n", rcvr, method, newnam)
@@ -1719,7 +1720,6 @@ func genwrapper(rcvr *Type, method *Field, newnam *Sym, iface int) {
 	fn.Func.Nname = newname(newnam)
 	fn.Func.Nname.Name.Defn = fn
 	fn.Func.Nname.Name.Param.Ntype = t
-	fn.Func.Nname.Sym.SetExported(true) // prevent export; see closure.go
 	declare(fn.Func.Nname, PFUNC)
 	funchdr(fn)
 
@@ -1923,14 +1923,6 @@ func implements(t, iface *Type, m, samename **Field, ptr *int) bool {
 		}
 	}
 
-	// We're going to emit an OCONVIFACE.
-	// Call itabname so that (t, iface)
-	// gets added to itabs early, which allows
-	// us to de-virtualize calls through this
-	// type/interface pair later. See peekitabs in reflect.go
-	if isdirectiface(t0) && !iface.IsEmptyInterface() {
-		itabname(t0, iface)
-	}
 	return true
 }
 

src/cmd/compile/internal/ssa/config.go

@@ -121,12 +121,6 @@ type Frontend interface {
 	SplitArray(LocalSlot) LocalSlot              // array must be length 1
 	SplitInt64(LocalSlot) (LocalSlot, LocalSlot) // returns (hi, lo)
 
-	// DerefItab dereferences an itab function
-	// entry, given the symbol of the itab and
-	// the byte offset of the function pointer.
-	// It may return nil.
-	DerefItab(sym *obj.LSym, offset int64) *obj.LSym
-
 	// Line returns a string describing the given position.
 	Line(src.XPos) string
 

src/cmd/compile/internal/ssa/export_test.go

@@ -112,7 +112,6 @@ func (d DummyFrontend) TypeInt() Type                                { return Ty
 func (d DummyFrontend) TypeUintptr() Type { return TypeUInt64 }
 func (d DummyFrontend) TypeString() Type  { panic("unimplemented") }
 func (d DummyFrontend) TypeBytePtr() Type { return TypeBytePtr }
-func (d DummyFrontend) DerefItab(sym *obj.LSym, off int64) *obj.LSym { return nil }
 
 func (d DummyFrontend) CanSSA(t Type) bool {
 	// There are no un-SSAable types in dummy land.

src/cmd/compile/internal/ssa/gen/generic.rules

@@ -1431,10 +1431,3 @@
 	&& c == config.ctxt.FixedFrameSize() + config.RegSize // offset of return value
 	&& warnRule(config.Debug_checknil() && v.Pos.Line() > 1, v, "removed nil check")
 	-> (Invalid)
-
-// De-virtualize interface calls into static calls.
-// Note that (ITab (IMake)) doesn't get
-// rewritten until after the first opt pass,
-// so this rule should trigger reliably.
-(InterCall [argsize] (Load (OffPtr [off] (ITab (IMake (Addr {itab} (SB)) _))) _) mem) && devirt(v, itab, off) != nil ->
-	(StaticCall [argsize] {devirt(v, itab, off)} mem)

src/cmd/compile/internal/ssa/rewrite.go

@@ -5,7 +5,6 @@
 package ssa
 
 import (
-	"cmd/internal/obj"
 	"crypto/sha1"
 	"fmt"
 	"math"
@@ -385,25 +384,6 @@ func uaddOvf(a, b int64) bool {
 	return uint64(a)+uint64(b) < uint64(a)
 }
 
-// de-virtualize an InterCall
-// 'sym' is the symbol for the itab
-func devirt(v *Value, sym interface{}, offset int64) *obj.LSym {
-	f := v.Block.Func
-	ext, ok := sym.(*ExternSymbol)
-	if !ok {
-		return nil
-	}
-	lsym := f.Config.Frontend().DerefItab(ext.Sym, offset)
-	if f.pass.debug > 0 {
-		if lsym != nil {
-			f.Config.Warnl(v.Pos, "de-virtualizing call")
-		} else {
-			f.Config.Warnl(v.Pos, "couldn't de-virtualize call")
-		}
-	}
-	return lsym
-}
-
 // isSamePtr reports whether p1 and p2 point to the same address.
 func isSamePtr(p1, p2 *Value) bool {
 	if p1 == p2 {

src/cmd/compile/internal/ssa/rewritegeneric.go

@@ -124,8 +124,6 @@ func rewriteValuegeneric(v *Value, config *Config) bool {
 		return rewriteValuegeneric_OpGreater8U(v, config)
 	case OpIMake:
 		return rewriteValuegeneric_OpIMake(v, config)
-	case OpInterCall:
-		return rewriteValuegeneric_OpInterCall(v, config)
 	case OpIsInBounds:
 		return rewriteValuegeneric_OpIsInBounds(v, config)
 	case OpIsNonNil:
@@ -5738,52 +5736,6 @@ func rewriteValuegeneric_OpIMake(v *Value, config *Config) bool {
 	}
 	return false
 }
-func rewriteValuegeneric_OpInterCall(v *Value, config *Config) bool {
-	b := v.Block
-	_ = b
-	// match: (InterCall [argsize] (Load (OffPtr [off] (ITab (IMake (Addr {itab} (SB)) _))) _) mem)
-	// cond: devirt(v, itab, off) != nil
-	// result: (StaticCall [argsize] {devirt(v, itab, off)} mem)
-	for {
-		argsize := v.AuxInt
-		v_0 := v.Args[0]
-		if v_0.Op != OpLoad {
-			break
-		}
-		v_0_0 := v_0.Args[0]
-		if v_0_0.Op != OpOffPtr {
-			break
-		}
-		off := v_0_0.AuxInt
-		v_0_0_0 := v_0_0.Args[0]
-		if v_0_0_0.Op != OpITab {
-			break
-		}
-		v_0_0_0_0 := v_0_0_0.Args[0]
-		if v_0_0_0_0.Op != OpIMake {
-			break
-		}
-		v_0_0_0_0_0 := v_0_0_0_0.Args[0]
-		if v_0_0_0_0_0.Op != OpAddr {
-			break
-		}
-		itab := v_0_0_0_0_0.Aux
-		v_0_0_0_0_0_0 := v_0_0_0_0_0.Args[0]
-		if v_0_0_0_0_0_0.Op != OpSB {
-			break
-		}
-		mem := v.Args[1]
-		if !(devirt(v, itab, off) != nil) {
-			break
-		}
-		v.reset(OpStaticCall)
-		v.AuxInt = argsize
-		v.Aux = devirt(v, itab, off)
-		v.AddArg(mem)
-		return true
-	}
-	return false
-}
 func rewriteValuegeneric_OpIsInBounds(v *Value, config *Config) bool {
 	b := v.Block
 	_ = b

test/devirt.go

@@ -1,64 +0,0 @@
-// errorcheck -0 -d=ssa/opt/debug=3
-
-package main
-
-import (
-	"crypto/sha1"
-	"errors"
-	"fmt"
-	"sync"
-)
-
-func f0() {
-	v := errors.New("error string")
-	_ = v.Error() // ERROR "de-virtualizing call$"
-}
-
-func f1() {
-	h := sha1.New()
-	buf := make([]byte, 4)
-	h.Write(buf)   // ERROR "de-virtualizing call$"
-	_ = h.Sum(nil) // ERROR "de-virtualizing call$"
-}
-
-func f2() {
-	// trickier case: make sure we see this is *sync.rlocker
-	// instead of *sync.RWMutex,
-	// even though they are the same pointers
-	var m sync.RWMutex
-	r := m.RLocker()
-
-	// deadlock if the type of 'r' is improperly interpreted
-	// as *sync.RWMutex
-	r.Lock() // ERROR "de-virtualizing call$"
-	m.RLock()
-	r.Unlock() // ERROR "de-virtualizing call$"
-	m.RUnlock()
-}
-
-type multiword struct{ a, b, c int }
-
-func (m multiword) Error() string { return fmt.Sprintf("%d, %d, %d", m.a, m.b, m.c) }
-
-func f3() {
-	// can't de-virtualize this one yet;
-	// it passes through a call to iconvT2I
-	var err error
-	err = multiword{1, 2, 3}
-	if err.Error() != "1, 2, 3" {
-		panic("bad call")
-	}
-
-	// ... but we can do this one
-	err = &multiword{1, 2, 3}
-	if err.Error() != "1, 2, 3" { // ERROR "de-virtualizing call$"
-		panic("bad call")
-	}
-}
-
-func main() {
-	f0()
-	f1()
-	f2()
-	f3()
-}