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

Limit bit-rot in the disabled backends #11217

Merged
merged 6 commits into from
May 3, 2022
Merged

Conversation

dra27
Copy link
Member

@dra27 dra27 commented Apr 24, 2022

While getting the other-checks stage to pass again in #11183, I disabled the CI checks for the presently disabled native code backends. While they may not be working, I think it's probably worth having make check_all_arches at least limit the bit-rot by ensuring that they continue to be compile, even if the code they produce is nonsense.

This PR:

Together, these changes restore make check_all_arches

The Iload constructor was changed from a triple to a record, but the
disabled backends weren't updated.
Eliminates the partial match warning in these backends, but obviously
the emitters are still broken.
Unused variable r12 was originally removed in PR#502.
Extra parameter added in the multicore merge. Obviously the emitter
remains broken.
The i386 emitter still had the 4.x name.
@dra27
Copy link
Member Author

dra27 commented Apr 24, 2022

I don't think we've made a final decision on the fate of the two 32-bit backends, which is why I've included them in the CI list for check_all_arches but it would also be reasonable to consider just adding power, s390x and riscv (i.e. the 64-bit architectures which we do intend restoring support for)

Copy link
Contributor

@nojb nojb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM: ensuring compilability of all backends is very good, and the changes won't cause hidden bugs whenever work on them resumes.

@gasche gasche merged commit 4bf5393 into ocaml:trunk May 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants