Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cmd/compile: defer wrapper has more instructions than hand-written wrapper #49656

Open
codefromthecrypt opened this issue Nov 18, 2021 · 2 comments
Open

cmd/compile: defer wrapper has more instructions than hand-written wrapper #49656

codefromthecrypt opened this issue Nov 18, 2021 · 2 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Unplanned

Comments

@codefromthecrypt
Copy link

defer src.Close() results in surprisingly more assembly than defer func() { _ = src.Close()}(). Is this a bug or am I missing something?

https://godbolt.org/z/KP4q1WTah

What version of Go are you using (go version)?

$ go version
go version go1.17.2 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/adrian/Library/Caches/go-build"
GOENV="/Users/adrian/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/adrian/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/adrian/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.17.2/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.17.2/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.17.2"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/adrian/tetratelabs/func-e/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/vd/1cf8zdb1721f4z5rjggy8bp40000gn/T/go-build3091093826=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Inspect the assembly generated from an implicitly swallowed error

func Close(src io.Closer) error {
    defer src.Close()
    return nil
}

vs doing it explicitly like this:

func Close(src io.Closer) error {
    defer func() { _ = src.Close()}()
    return nil
}

### What did you expect to see?

I expected to see similar size of assembly.

### What did you see instead?

The assembly of the first first was significantly larger https://godbolt.org/z/KP4q1WTah
@codefromthecrypt codefromthecrypt mentioned this issue Nov 18, 2021
Open
@randall77
Copy link
Contributor

It's 100 vs. 87 instructions.

I think most of the extra code is the required wrapper for defer+regabi. I'm not sure if there's any plan to simplify that code. @cherrymui

(The size comparison goes the opposite way for 1.16, because no wrapper function is required.)

@cherrymui
Copy link
Member

The auto-generated wrapper is essentially the same as the hand-written closure func() { src.Close() } except that being a wrapper, it has extra logic for panic adjustment when recovering a panic. We probably could skip that part if we know the callee doesn't recover panic.

@cherrymui cherrymui changed the title possibly bloated assembly for deferred functions that ignore errors cmd/compile: defer wrapper has more instructions than hand-written wrapper Nov 18, 2021
@cherrymui cherrymui added this to the Unplanned milestone Nov 18, 2021
@heschi heschi added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Nov 22, 2021
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@codefromthecrypt @randall77 @gopherbot @cherrymui @heschi