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

Remove float32/simd backend extension checks #2768

Merged
merged 2 commits into from
Jul 15, 2024
Merged

Conversation

TheNumbat
Copy link
Contributor

@TheNumbat TheNumbat commented Jul 10, 2024

We currently allow no-extension targets to depend on libraries that use extensions.
The compiler backend also asserts that the float32/SIMD extensions are enabled whenever it encounters a f32/v128 register.

After adding the float32 API to stdlib_stable, this has become a bigger problem.
When linking a no-extension target, the compiler can cross-module inline code from stdlib_stable, so the backend will find f32/v128 temps and assert.
This PR removes the assertions.

It's not clear whether this is the right solution - it might be possible to conditionally disable the checks for imported code, but that seems much harder.
At this point the f32/v128 code paths are pretty well tested, and I don't think we need to worry about catching bad types produced by the middle-end here, so I think it's ok to remove the checks. The regalloc validator can catch allocation issues in the cfg pipelines.

@TheNumbat TheNumbat requested a review from xclerc July 10, 2024 20:04
@goldfirere
Copy link
Collaborator

I haven’t reviewed the implementation of the change Max writes about, but I think this is fine. Having no-extension code depend on extension code is perhaps strange, but we have other mechanisms in place to stop us from doing this in a bad way; there’s no reason the compiler has to enforce this.

@TheNumbat TheNumbat merged commit 7967617 into main Jul 15, 2024
18 checks passed
@TheNumbat TheNumbat deleted the delete-backend-ext-checks branch July 15, 2024 13:58
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 this pull request may close these issues.

3 participants