golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.06k stars 17.55k forks source link

cmd/compile: elide useless type assertion #25189

Open dsnet opened 6 years ago

dsnet commented 6 years ago

Consider the following:

type CustomUnmarshaler interface {
    CustomUnmarshal([]byte) error
}

type MyMessage struct{}

func (m *MyMessage) Unmarshal(b []byte) error {
    if u, ok := (interface{})(m).(CustomUnmarshaler); ok {
        return u.CustomUnmarshal(b)
    }
    return nil
}

In this situation, the compiler knows that the MyMessage type has no CustomUnmarshal method, so the type assertion cannot possibly succeed. In this case, the condition of the if statement can be statically determined and the body be considered dead code.

However, a tip build of the compiler continues to emit code for the assertion.

0x0021 00033 (/tmp/main.go:10)  LEAQ    type."".CustomUnmarshaler(SB), AX
0x0028 00040 (/tmp/main.go:10)  MOVQ    AX, (SP)
0x002c 00044 (/tmp/main.go:10)  LEAQ    type.*"".Foo(SB), AX
0x0033 00051 (/tmp/main.go:10)  MOVQ    AX, 8(SP)
0x0038 00056 (/tmp/main.go:9)   MOVQ    "".f+64(SP), AX
0x003d 00061 (/tmp/main.go:10)  MOVQ    AX, 16(SP)
0x0042 00066 (/tmp/main.go:10)  PCDATA  $0, $1
0x0042 00066 (/tmp/main.go:10)  CALL    runtime.assertE2I2(SB)
0x0047 00071 (/tmp/main.go:10)  MOVQ    24(SP), AX
0x004c 00076 (/tmp/main.go:10)  MOVQ    32(SP), CX
0x0051 00081 (/tmp/main.go:10)  LEAQ    40(SP), DX
0x0056 00086 (/tmp/main.go:10)  CMPB    (DX), $0
0x0059 00089 (/tmp/main.go:10)  JEQ 161
0x005b 00091 (/tmp/main.go:11)  MOVQ    24(AX), AX
0x005f 00095 (/tmp/main.go:11)  MOVQ    "".b+72(SP), DX
0x0064 00100 (/tmp/main.go:11)  MOVQ    DX, 8(SP)
0x0069 00105 (/tmp/main.go:11)  MOVQ    "".b+80(SP), DX
0x006e 00110 (/tmp/main.go:11)  MOVQ    DX, 16(SP)
0x0073 00115 (/tmp/main.go:11)  MOVQ    "".b+88(SP), DX
0x0078 00120 (/tmp/main.go:11)  MOVQ    DX, 24(SP)
0x007d 00125 (/tmp/main.go:11)  MOVQ    CX, (SP)
0x0081 00129 (/tmp/main.go:11)  PCDATA  $0, $2
0x0081 00129 (/tmp/main.go:11)  CALL    AX
0x0083 00131 (/tmp/main.go:11)  MOVQ    32(SP), AX
0x0088 00136 (/tmp/main.go:11)  MOVQ    40(SP), CX
0x008d 00141 (/tmp/main.go:11)  MOVQ    AX, "".~r1+96(SP)
0x0092 00146 (/tmp/main.go:11)  MOVQ    CX, "".~r1+104(SP)
0x0097 00151 (/tmp/main.go:11)  MOVQ    48(SP), BP
0x009c 00156 (/tmp/main.go:11)  ADDQ    $56, SP
0x00a0 00160 (/tmp/main.go:11)  RET
rasky commented 6 years ago

I assume these things happen while inlining (and as we get more aggressive in inlining, they will be more and more common).

The type conversion is lowered during buildssa into runtime calls. I've already attempted to write some code to delay lowering in a similar case, but I couldn't find a good way to layout a runtime function call after buildssa (eg: stack slots). It's way above my head.

It would help if somebody more familiar with the compiler could come up with some utility to add a function call during a SSA pass. That would be very useful for me to start working towards delaying lowering of runtime calls, exploiting optimizations opportunities that higher-level semantics bring.

bcmills commented 6 years ago

How much real-world code actually does these impossible assertions? (That is, what's the potential impact of the optimization?)

I assume that this mostly occurs in generated code, but in that case wouldn't it be even better — particularly in terms of compile times — to avoid emitting the redundant assertions in the first place?

CAFxX commented 6 years ago

@bcmills the point that @rasky makes about this happening because of inlining I think is probably the main source of these occurrences already today. See e.g. https://godbolt.org/g/GzDvaU or https://godbolt.org/g/N5W6Ua (in both cases the call to f() is inlined, but the type assertion remains)

(Also, but this is a vastly less important argument in the context of this discussion, requiring code generators to be that smart would vastly complicate them)

navytux commented 6 years ago

Tracking concrete type behind an interface will also help https://github.com/golang/go/issues/19361, thus lowering call overhead if methods are invoked, and potentially lowering GC overhead through not letting call arguments to escape.

rasky commented 6 years ago

I think that, wrt inlining, we're in a sort of catch-22: inlining does not seem to help much because we're bad at exploiting many of these opportunities in our optimizers; and adding optimizations like this one is going to have a tool speed impact without great benefits because we don't inline nearly as much as we should.