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: Register all functions in test file for interpreter #4817

Merged
merged 3 commits into from
Aug 30, 2022

Conversation

afonso360
Copy link
Contributor

@afonso360 afonso360 commented Aug 30, 2022

👋 Hey,

Lets try this again! The issue in #4800 was due to the signature checking that we included in #4782 being overly strict.

In the interpreter our DataValues do not have full type information about their data. We only have a single boolean representation B instead of one for each size b1/b8/b16/etc.., this caused a error in the new call.clif tests since when we query the types of these DataValues we just return a "Default" type. Which in this case was b8 and different from b1 in the signature, therefore a signature error!

The solution in the last commit is to relax our signature checking constraints for these two type categories (bools & vectors).

cc: #4800
cc: #4810

@github-actions github-actions bot added the cranelift Issues related to the Cranelift code generator label Aug 30, 2022
Copy link
Contributor

@jameysharp jameysharp left a comment

Choose a reason for hiding this comment

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

Looks good to me. And just to be sure, I ran the test suite after locally merging with main, and it still passed. 😁

@jameysharp jameysharp merged commit 3ce3eeb into bytecodealliance:main Aug 30, 2022
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
Development

Successfully merging this pull request may close these issues.

2 participants