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

cranelift: Fix invalid trampoline when returning boolean vectors #3341

Closed

Conversation

afonso360
Copy link
Contributor

@afonso360 afonso360 commented Sep 13, 2021

Hey,

This is a extension of #3306. In that PR we accidentally only dealt with receiving bool vectors as arguments to the callee function. This caused issues in #3332 where those tests were failing to run on the real backends, due to the generated trampoline being invalid when the callee returns bool vectors.

For more context about this, see this comment with the invalid trampoline, and the rest of the thread with a proposed solution.

The solution here is not great, bint with the correct type would fix this, but is not implemented in some backends, same with bmask. raw_bitcast solves the issue, but we really should fix bmask/bint and use that.

Fixing trampoline bool args is tracked in #2237

Fixes #3334
Fixes #3335

@afonso360
Copy link
Contributor Author

See also @akirilov-arm 's comments on #3335:

... we can work around this by raw_bitcast'ing it into the appropriate integer type.

No, that's wrong (but it's probably going to work) - I don't think we agreed in #3205 to change the representation of vector >booleans. This would be an appropriate work-around for Bmask (but then we have #1429).

Sorry, I just missed your comment before opening this!

Just to reiterate, I know raw_bitcast is not the right answer. And agree that bmask would be a better solution

@github-actions github-actions bot added the cranelift Issues related to the Cranelift code generator label Sep 13, 2021
@afonso360
Copy link
Contributor Author

Closing this in favour of implementing bmask in AArch64 and x64, and using that. Should have a PR soon-ish.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift Issues related to the Cranelift code generator
Projects
None yet
1 participant