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: instruction reordering with optimizations disabled (go1.21) #58482
Labels
compiler/runtime
Issues related to the Go compiler and/or runtime.
FrozenDueToAge
NeedsInvestigation
Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Comments
gopherbot
added
the
compiler/runtime
Issues related to the Go compiler and/or runtime.
label
Feb 12, 2023
cc @golang/compiler |
prattmic
added
the
NeedsInvestigation
Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
label
Feb 14, 2023
Change https://go.dev/cl/468455 mentions this issue: |
Change https://go.dev/cl/468615 mentions this issue: |
Change https://go.dev/cl/468695 mentions this issue: |
gopherbot
pushed a commit
that referenced
this issue
Feb 16, 2023
The debug info generation currently depends on this invariant. A small update to CL 468455. Update #58482 Change-Id: Ica305d360d9af04036c604b6a65b683f7cb6e212 Reviewed-on: https://go-review.googlesource.com/c/go/+/468695 Run-TryBot: Keith Randall <khr@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: David Chase <drchase@google.com>
gopherbot
pushed a commit
that referenced
this issue
Feb 17, 2023
…pointer For c + nil, we want the result to still be of pointer type. Fixes ppc64le build failure with CL 468455, in issue33724.go. The problem in that test is that it requires a nil check to be scheduled before the corresponding load. This normally happens fine because we prioritize nil checks. If we have nilcheck(p) and load(p), once p is scheduled the nil check will always go before the load. The issue we saw in 33724 is that when p is a nil pointer, we ended up with two different p's, an int64(0) as the argument to the nil check and an (*Outer)(0) as the argument to the load. Those two zeroes don't get CSEd, so if the (*Outer)(0) happens to get scheduled first, the load can end up before the nilcheck. Fix this by always having constant arithmetic preserve the pointerness of the value, so that both zeroes are of type *Outer and get CSEd. Update #58482 Update #33724 Change-Id: Ib9b8c0446f1690b574e0f3c0afb9934efbaf3513 Reviewed-on: https://go-review.googlesource.com/c/go/+/468615 Reviewed-by: Keith Randall <khr@google.com> Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: David Chase <drchase@google.com> TryBot-Bypass: Keith Randall <khr@golang.org>
johanbrandhorst
pushed a commit
to Pryz/go
that referenced
this issue
Feb 22, 2023
The actual scheduling of SP early doesn't really matter, but lots of early spills (of arguments) depend on SP so they can't schedule until SP does. Fixes golang#58482 Change-Id: Ie581fba7cb173d665c11f797f39d824b1c040a2d Reviewed-on: https://go-review.googlesource.com/c/go/+/468455 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Alessandro Arzilli <alessandro.arzilli@gmail.com> Reviewed-by: Keith Randall <khr@google.com> Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: David Chase <drchase@google.com>
johanbrandhorst
pushed a commit
to Pryz/go
that referenced
this issue
Feb 22, 2023
The debug info generation currently depends on this invariant. A small update to CL 468455. Update golang#58482 Change-Id: Ica305d360d9af04036c604b6a65b683f7cb6e212 Reviewed-on: https://go-review.googlesource.com/c/go/+/468695 Run-TryBot: Keith Randall <khr@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: David Chase <drchase@google.com>
johanbrandhorst
pushed a commit
to Pryz/go
that referenced
this issue
Feb 22, 2023
…pointer For c + nil, we want the result to still be of pointer type. Fixes ppc64le build failure with CL 468455, in issue33724.go. The problem in that test is that it requires a nil check to be scheduled before the corresponding load. This normally happens fine because we prioritize nil checks. If we have nilcheck(p) and load(p), once p is scheduled the nil check will always go before the load. The issue we saw in 33724 is that when p is a nil pointer, we ended up with two different p's, an int64(0) as the argument to the nil check and an (*Outer)(0) as the argument to the load. Those two zeroes don't get CSEd, so if the (*Outer)(0) happens to get scheduled first, the load can end up before the nilcheck. Fix this by always having constant arithmetic preserve the pointerness of the value, so that both zeroes are of type *Outer and get CSEd. Update golang#58482 Update golang#33724 Change-Id: Ib9b8c0446f1690b574e0f3c0afb9934efbaf3513 Reviewed-on: https://go-review.googlesource.com/c/go/+/468615 Reviewed-by: Keith Randall <khr@google.com> Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: David Chase <drchase@google.com> TryBot-Bypass: Keith Randall <khr@golang.org>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
compiler/runtime
Issues related to the Go compiler and/or runtime.
FrozenDueToAge
NeedsInvestigation
Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
The function main.Foo in teststepconcurrent.go compiled with
-gcflags=-N -l
now produces:The block of instructions assigned to the function header (:13) after the (LEAQ) addition is unfortunate.
Before it used to be:
This was introduced by 12befc3 (cmd/compile: improve scheduling pass). cc @randall77 @dr2chase
The text was updated successfully, but these errors were encountered: