Skip to content

Commit de9da0d

Browse files
mateusz834gopherbot
authored andcommitted
cmd/compile/internal/devirtualize: improve concrete type analysis
This change improves the concrete type analysis in the devirtualizer, it not longer relies on ir.Reassigned, it now statically tries to determine the concrete type of an interface, even when assigned multiple times, following type assertions and iface conversions. Alternative to CL 649195 Updates golang#69521 Fixes golang#64824 Change-Id: Ib1656e19f3619ab2e1e6b2c78346cc320490b2af GitHub-Last-Rev: e8fa0b1 GitHub-Pull-Request: golang#71935 Reviewed-on: https://go-review.googlesource.com/c/go/+/652036 Reviewed-by: Michael Pratt <[email protected]> Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Keith Randall <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Keith Randall <[email protected]>
1 parent ae094a1 commit de9da0d

File tree

12 files changed

+2039
-35
lines changed

12 files changed

+2039
-35
lines changed

src/cmd/compile/internal/devirtualize/devirtualize.go

Lines changed: 452 additions & 11 deletions
Large diffs are not rendered by default.

src/cmd/compile/internal/inline/interleaved/interleaved.go

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ func DevirtualizeAndInlinePackage(pkg *ir.Package, profile *pgoir.Profile) {
4545
inlState := make(map[*ir.Func]*inlClosureState)
4646
calleeUseCounts := make(map[*ir.Func]int)
4747

48+
var state devirtualize.State
49+
4850
// Pre-process all the functions, adding parentheses around call sites and starting their "inl state".
4951
for _, fn := range typecheck.Target.Funcs {
5052
bigCaller := base.Flag.LowerL != 0 && inline.IsBigFunc(fn)
@@ -58,7 +60,7 @@ func DevirtualizeAndInlinePackage(pkg *ir.Package, profile *pgoir.Profile) {
5860

5961
// Do a first pass at counting call sites.
6062
for i := range s.parens {
61-
s.resolve(i)
63+
s.resolve(&state, i)
6264
}
6365
}
6466

@@ -102,10 +104,11 @@ func DevirtualizeAndInlinePackage(pkg *ir.Package, profile *pgoir.Profile) {
102104
for {
103105
for i := l0; i < l1; i++ { // can't use "range parens" here
104106
paren := s.parens[i]
105-
if new := s.edit(i); new != nil {
107+
if origCall, inlinedCall := s.edit(&state, i); inlinedCall != nil {
106108
// Update AST and recursively mark nodes.
107-
paren.X = new
108-
ir.EditChildren(new, s.mark) // mark may append to parens
109+
paren.X = inlinedCall
110+
ir.EditChildren(inlinedCall, s.mark) // mark may append to parens
111+
state.InlinedCall(s.fn, origCall, inlinedCall)
109112
done = false
110113
}
111114
}
@@ -114,7 +117,7 @@ func DevirtualizeAndInlinePackage(pkg *ir.Package, profile *pgoir.Profile) {
114117
break
115118
}
116119
for i := l0; i < l1; i++ {
117-
s.resolve(i)
120+
s.resolve(&state, i)
118121
}
119122

120123
}
@@ -188,7 +191,7 @@ type inlClosureState struct {
188191
// resolve attempts to resolve a call to a potentially inlineable callee
189192
// and updates use counts on the callees. Returns the call site count
190193
// for that callee.
191-
func (s *inlClosureState) resolve(i int) (*ir.Func, int) {
194+
func (s *inlClosureState) resolve(state *devirtualize.State, i int) (*ir.Func, int) {
192195
p := s.parens[i]
193196
if i < len(s.resolved) {
194197
if callee := s.resolved[i]; callee != nil {
@@ -200,7 +203,7 @@ func (s *inlClosureState) resolve(i int) (*ir.Func, int) {
200203
if !ok { // previously inlined
201204
return nil, -1
202205
}
203-
devirtualize.StaticCall(call)
206+
devirtualize.StaticCall(state, call)
204207
if callee := inline.InlineCallTarget(s.fn, call, s.profile); callee != nil {
205208
for len(s.resolved) <= i {
206209
s.resolved = append(s.resolved, nil)
@@ -213,23 +216,23 @@ func (s *inlClosureState) resolve(i int) (*ir.Func, int) {
213216
return nil, 0
214217
}
215218

216-
func (s *inlClosureState) edit(i int) ir.Node {
219+
func (s *inlClosureState) edit(state *devirtualize.State, i int) (*ir.CallExpr, *ir.InlinedCallExpr) {
217220
n := s.parens[i].X
218221
call, ok := n.(*ir.CallExpr)
219222
if !ok {
220-
return nil
223+
return nil, nil
221224
}
222225
// This is redundant with earlier calls to
223226
// resolve, but because things can change it
224227
// must be re-checked.
225-
callee, count := s.resolve(i)
228+
callee, count := s.resolve(state, i)
226229
if count <= 0 {
227-
return nil
230+
return nil, nil
228231
}
229232
if inlCall := inline.TryInlineCall(s.fn, call, s.bigCaller, s.profile, count == 1 && callee.ClosureParent != nil); inlCall != nil {
230-
return inlCall
233+
return call, inlCall
231234
}
232-
return nil
235+
return nil, nil
233236
}
234237

235238
// Mark inserts parentheses, and is called repeatedly.
@@ -338,16 +341,18 @@ func (s *inlClosureState) unparenthesize() {
338341
// returns.
339342
func (s *inlClosureState) fixpoint() bool {
340343
changed := false
344+
var state devirtualize.State
341345
ir.WithFunc(s.fn, func() {
342346
done := false
343347
for !done {
344348
done = true
345349
for i := 0; i < len(s.parens); i++ { // can't use "range parens" here
346350
paren := s.parens[i]
347-
if new := s.edit(i); new != nil {
351+
if origCall, inlinedCall := s.edit(&state, i); inlinedCall != nil {
348352
// Update AST and recursively mark nodes.
349-
paren.X = new
350-
ir.EditChildren(new, s.mark) // mark may append to parens
353+
paren.X = inlinedCall
354+
ir.EditChildren(inlinedCall, s.mark) // mark may append to parens
355+
state.InlinedCall(s.fn, origCall, inlinedCall)
351356
done = false
352357
changed = true
353358
}

src/cmd/compile/internal/ir/expr.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -677,6 +677,11 @@ type TypeAssertExpr struct {
677677

678678
// An internal/abi.TypeAssert descriptor to pass to the runtime.
679679
Descriptor *obj.LSym
680+
681+
// When set to true, if this assert would panic, then use a nil pointer panic
682+
// instead of an interface conversion panic.
683+
// It must not be set for type asserts using the commaok form.
684+
UseNilPanic bool
680685
}
681686

682687
func NewTypeAssertExpr(pos src.XPos, x Node, typ *types.Type) *TypeAssertExpr {

src/cmd/compile/internal/noder/reader.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2961,6 +2961,7 @@ func (r *reader) multiExpr() []ir.Node {
29612961
as.Def = true
29622962
for i := range results {
29632963
tmp := r.temp(pos, r.typ())
2964+
tmp.Defn = as
29642965
as.PtrInit().Append(ir.NewDecl(pos, ir.ODCL, tmp))
29652966
as.Lhs.Append(tmp)
29662967

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5827,6 +5827,25 @@ func (s *state) dottype(n *ir.TypeAssertExpr, commaok bool) (res, resok *ssa.Val
58275827
if n.ITab != nil {
58285828
targetItab = s.expr(n.ITab)
58295829
}
5830+
5831+
if n.UseNilPanic {
5832+
if commaok {
5833+
base.Fatalf("unexpected *ir.TypeAssertExpr with UseNilPanic == true && commaok == true")
5834+
}
5835+
if n.Type().IsInterface() {
5836+
// Currently we do not expect the compiler to emit type asserts with UseNilPanic, that assert to an interface type.
5837+
// If needed, this can be relaxed in the future, but for now we can assert that.
5838+
base.Fatalf("unexpected *ir.TypeAssertExpr with UseNilPanic == true && Type().IsInterface() == true")
5839+
}
5840+
typs := s.f.Config.Types
5841+
iface = s.newValue2(
5842+
ssa.OpIMake,
5843+
iface.Type,
5844+
s.nilCheck(s.newValue1(ssa.OpITab, typs.BytePtr, iface)),
5845+
s.newValue1(ssa.OpIData, typs.BytePtr, iface),
5846+
)
5847+
}
5848+
58305849
return s.dottype1(n.Pos(), n.X.Type(), n.Type(), iface, nil, target, targetItab, commaok, n.Descriptor)
58315850
}
58325851

src/crypto/sha256/sha256_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -471,3 +471,17 @@ func BenchmarkHash256K(b *testing.B) {
471471
func BenchmarkHash1M(b *testing.B) {
472472
benchmarkSize(b, 1024*1024)
473473
}
474+
475+
func TestAllocatonsWithTypeAsserts(t *testing.T) {
476+
cryptotest.SkipTestAllocations(t)
477+
allocs := testing.AllocsPerRun(100, func() {
478+
h := New()
479+
h.Write([]byte{1, 2, 3})
480+
marshaled, _ := h.(encoding.BinaryMarshaler).MarshalBinary()
481+
marshaled, _ = h.(encoding.BinaryAppender).AppendBinary(marshaled[:0])
482+
h.(encoding.BinaryUnmarshaler).UnmarshalBinary(marshaled)
483+
})
484+
if allocs != 0 {
485+
t.Fatalf("allocs = %v; want = 0", allocs)
486+
}
487+
}

src/runtime/pprof/pprof_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,11 @@ func (h inlineWrapper) dump(pcs []uintptr) {
344344

345345
func inlinedWrapperCallerDump(pcs []uintptr) {
346346
var h inlineWrapperInterface
347+
348+
// Take the address of h, such that h.dump() call (below)
349+
// does not get devirtualized by the compiler.
350+
_ = &h
351+
347352
h = &inlineWrapper{}
348353
h.dump(pcs)
349354
}

0 commit comments

Comments
 (0)