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.

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: checkbce failing in 'noopt' builder #27684

Closed
bcmills opened this issue Sep 14, 2018 · 4 comments
Closed

cmd/compile: checkbce failing in 'noopt' builder #27684

bcmills opened this issue Sep 14, 2018 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Sep 14, 2018

Example: https://build.golang.org/log/15361a99186b257b1df1e9f95fa505b3ffcc1a66

##### ../test
# go run run.go -- checkbce.go

checkbce.go:164: missing error "Found IsInBounds$"
checkbce.go:166: missing error "Found IsInBounds$"
checkbce.go:168: missing error "Found IsInBounds$"
Unmatched Errors:
checkbce.go:165: Found IsSliceInBounds
checkbce.go:167: Found IsSliceInBounds
checkbce.go:169: Found IsSliceInBounds

FAIL	checkbce.go	0.061s

CC: @randall77 @josharian @martisch

@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Sep 14, 2018
@bcmills bcmills added this to the Go1.12 milestone Sep 14, 2018
@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 14, 2018
@gopherbot gopherbot removed the NeedsFix The path to resolution is known, but the work has not been done. label Sep 14, 2018
@bcmills bcmills added the Testing An issue that has been verified to require only test changes, not just a test failure. label Sep 14, 2018
@randall77
Copy link
Contributor

I'm confused by this. Shouldn't we not be testing bounds check elimination on the noopt builder?
The prove pass certainly isn't running on this builder.
There may be cases where the noopt builder removes a bounds check, but it certainly won't be as aggressive as the standard builder with all the bounds check machinery enabled.

@josharian
Copy link
Contributor

See also discussion at https://go-review.googlesource.com/107355 which is when this started failing.

@randall77
Copy link
Contributor

I guess somehow GO_GCFLAGS isn't making to the recursive calls to go tool compile that the test harness does.
Which may be a good thing, otherwise lots of things in test would fail.
But that leaves the conundrum of why this test is failing on the noopt builder.

@randall77
Copy link
Contributor

This was fixed when #27833 was fixed.

@golang golang locked and limited conversation to collaborators Nov 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Development

No branches or pull requests

4 participants