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

Fix build, for real this time #502

Merged
merged 5 commits into from
Nov 5, 2023
Merged

Fix build, for real this time #502

merged 5 commits into from
Nov 5, 2023

Conversation

tilk
Copy link
Member

@tilk tilk commented Nov 3, 2023

Always run the affected workflow before merging. Always run the affected workflow before merging. Always run the affected workflow before merging. Always run the affected workflow before merging.

Summary of the comments: UnalignedFetcher sometimes fetches one additional cache line beyond the ones that contain the code. Not really a big problem, but the core freezes on the bus error that can then happen. To work around this issue, an additional cache line is added to code segments.

@tilk tilk added the bug Something isn't working label Nov 3, 2023
@tilk tilk marked this pull request as draft November 3, 2023 09:17
@tilk tilk force-pushed the tilk/fix-build-mem-2 branch from e36cd38 to 2ccd9d7 Compare November 3, 2023 09:41
@tilk
Copy link
Member Author

tilk commented Nov 3, 2023

Looks like this try was misguided. I objdumped the failing tests, and it looks like the core is still somehow trying to fetch data just outside the code segment. In both test programs, the last instruction is compressed ret, which is very close to the cache line boundary. I believe it's the UnalignedFetcher which makes the additional, unneeded request.

@tilk
Copy link
Member Author

tilk commented Nov 3, 2023

I think that the right way to proceed is to stop crashing the simulation when accessing stuff outside of the defined memory ranges. The UnalignedFetcher does some speculative prefetching, and prefetching outside of a memory range is not a failure.

@tilk
Copy link
Member Author

tilk commented Nov 3, 2023

This didn't help, either. The core freezes. Looks like fixing this properly requires some proper debugging. I think that, for now, we should just work around the issue.

@tilk tilk force-pushed the tilk/fix-build-mem-2 branch from d428f1d to 7d45ab2 Compare November 3, 2023 11:50
@tilk tilk marked this pull request as ready for review November 3, 2023 12:08
@tilk
Copy link
Member Author

tilk commented Nov 3, 2023

Finally, benchmarks work again. The temporary workaround is to add one more cache line to the code segment.

@tilk tilk merged commit bf83d82 into master Nov 5, 2023
6 checks passed
@tilk tilk deleted the tilk/fix-build-mem-2 branch November 5, 2023 21:11
github-actions bot pushed a commit that referenced this pull request Nov 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants