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: Value live at entry [1.19 backport] #54917
Comments
Deferring approval until we can decide whether this should also be applied to 1.18. @mdempsky said in the non-backport issue:
Does this apply to 1.18 as well? How does this interact with the no-backports-of-generic-fixes policy to 1.18? Thanks! |
Hmm, to be clear, the original issue happens since go1.18, so this is not a regression at all. |
I've written my thoughts on backporting this fix at #54911 (comment). |
@mdempsky Thanks! Since the fix is safe and the workaround is onerous (and applies to both proposed backports), I'm inclined to approve. |
Change https://go.dev/cl/429896 mentions this issue: |
Closed by merging 225bcec to release-branch.go1.19. |
…eEffects issue ir.ClosureExpr implements ir.InitNode, so ir.InitExpr can prepend init statements to it. However, CalleeEffects wasn't aware of this and could cause the init statements to get dropped when inlining a call to a closure. This isn't an issue today, because we don't create closures with init statements. But I ran into this within unified IR. Easy and robust solution: just take advantage that ir.TakeInit can handle any node. Fixes #54917. Change-Id: Ica05fbf6a8c5be4b11927daf84491a1140da5431 Reviewed-on: https://go-review.googlesource.com/c/go/+/422196 Reviewed-by: Than McIntosh <thanm@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-on: https://go-review.googlesource.com/c/go/+/429896 Reviewed-by: Michael Knyszek <mknyszek@google.com>
…eEffects issue ir.ClosureExpr implements ir.InitNode, so ir.InitExpr can prepend init statements to it. However, CalleeEffects wasn't aware of this and could cause the init statements to get dropped when inlining a call to a closure. This isn't an issue today, because we don't create closures with init statements. But I ran into this within unified IR. Easy and robust solution: just take advantage that ir.TakeInit can handle any node. Fixes golang#54917. Change-Id: Ica05fbf6a8c5be4b11927daf84491a1140da5431 Reviewed-on: https://go-review.googlesource.com/c/go/+/422196 Reviewed-by: Than McIntosh <thanm@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-on: https://go-review.googlesource.com/c/go/+/429896 Reviewed-by: Michael Knyszek <mknyszek@google.com>
…eEffects issue ir.ClosureExpr implements ir.InitNode, so ir.InitExpr can prepend init statements to it. However, CalleeEffects wasn't aware of this and could cause the init statements to get dropped when inlining a call to a closure. This isn't an issue today, because we don't create closures with init statements. But I ran into this within unified IR. Easy and robust solution: just take advantage that ir.TakeInit can handle any node. Fixes golang#54917. Change-Id: Ica05fbf6a8c5be4b11927daf84491a1140da5431 Reviewed-on: https://go-review.googlesource.com/c/go/+/422196 Reviewed-by: Than McIntosh <thanm@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-on: https://go-review.googlesource.com/c/go/+/429896 Reviewed-by: Michael Knyszek <mknyszek@google.com>
…eEffects issue ir.ClosureExpr implements ir.InitNode, so ir.InitExpr can prepend init statements to it. However, CalleeEffects wasn't aware of this and could cause the init statements to get dropped when inlining a call to a closure. This isn't an issue today, because we don't create closures with init statements. But I ran into this within unified IR. Easy and robust solution: just take advantage that ir.TakeInit can handle any node. Fixes golang#54917. Change-Id: Ica05fbf6a8c5be4b11927daf84491a1140da5431 Reviewed-on: https://go-review.googlesource.com/c/go/+/422196 Reviewed-by: Than McIntosh <thanm@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-on: https://go-review.googlesource.com/c/go/+/429896 Reviewed-by: Michael Knyszek <mknyszek@google.com>
@cuonglm requested issue #54911 to be considered for backport to the next 1.19 minor release.
The text was updated successfully, but these errors were encountered: