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

Comments

Projects
None yet
4 participants
@bcmills
Copy link
Member

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 label Sep 14, 2018

@bcmills bcmills added this to the Go1.12 milestone Sep 14, 2018

@gopherbot gopherbot removed the NeedsFix label Sep 14, 2018

@bcmills bcmills added the Testing label Sep 14, 2018

@randall77

This comment has been minimized.

Copy link
Contributor

commented Sep 14, 2018

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

This comment has been minimized.

Copy link
Contributor

commented Sep 14, 2018

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

@randall77

This comment has been minimized.

Copy link
Contributor

commented Sep 14, 2018

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

This comment has been minimized.

Copy link
Contributor

commented Nov 26, 2018

This was fixed when #27833 was fixed.

@randall77 randall77 closed this Nov 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.