-
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: function inlining produces incorrect results in certain conditions [1.12 backport] #30567
Comments
Change https://golang.org/cl/165858 mentions this issue: |
@mvdan - there isn't a reason provided in the gopherbot message. Would you mind providing one for this backport? |
There's an underlying bug which is fixed with the CL linked above. That bug has been in existence since at least 1.2. |
Closed by merging f062f48 to release-branch.go1.12. |
…g ops Make sure the side effects inside short-circuited operations (&& and ||) happen correctly. Before this CL, we attached the side effects to the node itself using exprInPlace. That caused other side effects in sibling expressions to get reordered with respect to the short circuit side effect. Instead, rewrite a && b like: r := a if r { r = b } That code we can keep correctly ordered with respect to other side-effects extracted from part of a big expression. exprInPlace seems generally unsafe. But this was the only case where exprInPlace is called not at the top level of an expression, so I don't think the other uses can actually trigger an issue (there can't be a sibling expression). TODO: maybe those cases don't need "in place", and we can retire that function generally. This CL needed a small tweak to the SSA generation of OIF so that the short circuit optimization still triggers. The short circuit optimization looks for triangle but not diamonds, so don't bother allocating a block if it will be empty. Go 1 benchmarks are in the noise. Fixes #30567 Change-Id: I19c04296bea63cbd6ad05f87a63b005029123610 Reviewed-on: https://go-review.googlesource.com/c/go/+/165617 Run-TryBot: Keith Randall <khr@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com> (cherry picked from commit 4a9064e) Reviewed-on: https://go-review.googlesource.com/c/go/+/165858 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@mvdan requested issue #30566 to be considered for backport to the next 1.12 minor release.
The text was updated successfully, but these errors were encountered: