-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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.
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
cmd/compile: functions with switches cannot be inlined #13071
Comments
This would be nice as part of a general revisit of inlining decisions. I don't think this is on anyone's radar at the moment, though. See #12312 We'll need noinline directives first so we can stop using switch{} as a noinline hint (should happen soon). |
The compiler can do a fine job, and can also inline it. From Jeremy Jackins's observation and rsc's recommendation in thread: "Pure Go math.Abs outperforms assembly version" https://groups.google.com/forum/#!topic/golang-dev/nP5mWvwAXZo Updates #13095 Change-Id: I3066f8eaa327bb403173b29791cc8661d7c0532c Reviewed-on: https://go-review.googlesource.com/16444 Reviewed-by: Ian Lance Taylor <iant@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
CL https://golang.org/cl/16446 mentions this issue. |
The bug number was a typo, and I forgot to switch the implementation back to if statements after the change from Float64bits in the first patchset back to branching. if statements can currently be inlined, but switch cannot (#13071) Change-Id: I81d0cf64bda69186c3d747a07047f6a694f8fa70 Reviewed-on: https://go-review.googlesource.com/16446 Reviewed-by: Robert Griesemer <gri@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
CL https://golang.org/cl/17171 mentions this issue. |
A bunch of tests until recently used switch{} to disable inlining. We just added a go:noinline directive to replace these uses. We'll need to make sure we replace all of the switch{} uses and then we can fix this bug. |
That sounds wonderful. Would that also fix #7655? And would this make it into go1.6? Regardless, I'd be happy to abandon that CL of mine. |
@randall77 You mentioned that it might be a good time to revisit inline decisions. Is that taking placing somewhere? I would be interested in being involved in that discussion. I've been working on trying to make compression libraries more performant, and many of the changes I make are related to coercing the compiler to inline certain functions. @klauspost would probably be interested in these discussions as well. |
I know of no one working on inline decisions right now. |
Functions with switches (#13071) cannot be inlined. Functions with consts (#7655) cannot be inlined. benchmark old MB/s new MB/s speedup BenchmarkEncodeDigitsSpeed1e4-4 10.25 10.20 1.00x BenchmarkEncodeDigitsSpeed1e5-4 26.44 27.22 1.03x BenchmarkEncodeDigitsSpeed1e6-4 32.28 33.51 1.04x BenchmarkEncodeDigitsDefault1e4-4 8.61 8.74 1.02x BenchmarkEncodeDigitsDefault1e5-4 7.03 6.98 0.99x BenchmarkEncodeDigitsDefault1e6-4 6.47 6.46 1.00x BenchmarkEncodeDigitsCompress1e4-4 8.62 8.73 1.01x BenchmarkEncodeDigitsCompress1e5-4 7.01 6.98 1.00x BenchmarkEncodeDigitsCompress1e6-4 6.43 6.53 1.02x BenchmarkEncodeTwainSpeed1e4-4 9.67 10.16 1.05x BenchmarkEncodeTwainSpeed1e5-4 26.46 26.94 1.02x BenchmarkEncodeTwainSpeed1e6-4 33.19 34.02 1.03x BenchmarkEncodeTwainDefault1e4-4 8.12 8.37 1.03x BenchmarkEncodeTwainDefault1e5-4 8.22 8.21 1.00x BenchmarkEncodeTwainDefault1e6-4 8.10 8.13 1.00x BenchmarkEncodeTwainCompress1e4-4 8.24 8.39 1.02x BenchmarkEncodeTwainCompress1e5-4 6.51 6.58 1.01x BenchmarkEncodeTwainCompress1e6-4 6.16 6.13 1.00x Change-Id: Ibafa5e3e2de0529853b5b3180e6fd6cb7090b76f Reviewed-on: https://go-review.googlesource.com/17171 Reviewed-by: Russ Cox <rsc@golang.org>
CL https://golang.org/cl/20824 mentions this issue. |
Using
go1.5
Consider the following package:
Using
go build -gcflags=-S dict.go
, I see that LastBytesA and LastBytesB produce the exact same assembly output.However,
go build -gcflags=-m dict.go
reports that only LastByteA can be inlined and not LastBytesB:I expect both LastBytesA and LastBytesB to be inline-able if they produce the same assembly output. This would be nice since (IMHO), switch statements are cleaner for some if-elseif-elseif-elseif-else blocks.
The text was updated successfully, but these errors were encountered: