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

Make the scheduling pass aware of the memory model #12216

Closed
kayceesrk opened this issue May 1, 2023 · 2 comments · Fixed by #12248
Closed

Make the scheduling pass aware of the memory model #12216

kayceesrk opened this issue May 1, 2023 · 2 comments · Fixed by #12248
Assignees

Comments

@kayceesrk
Copy link
Contributor

kayceesrk commented May 1, 2023

This is a placeholder issue in order to track the interaction of the scheduling pass and the memory model

Yes, we should fix the scheduling pass. From a quick look, it suffices to classify atomic loads like store instructions, thus preventing any reordering with other instructions that access memory. (Instructions classified as loads can be reordered with other instructions classified as loads, but not with stores.)

from #11712 (comment).

Either we disable the scheduling pass completely on OCaml 5 or we make the scheduling pass aware of the reordering allowed by the OCaml memory model.

The scheduling pass is currently disabled for x86_64 and Arm64, and #11712 disables the scheduling pass for s390x.

What about RISC-V? @nojb.

@nojb
Copy link
Contributor

nojb commented May 5, 2023

What about RISC-V? @nojb.

You have probably already noticed, but scheduling is also disabled in RISC-V.

@kayceesrk
Copy link
Contributor Author

Thanks for the confirmation @nojb.

@gasche gasche self-assigned this May 8, 2023
xavierleroy added a commit to xavierleroy/ocaml that referenced this issue May 16, 2023
xavierleroy added a commit that referenced this issue May 16, 2023
Just treat atomic loads like stores.

Fixes: #12216
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants