Skip to content

cranelift: Disable i128 divs on fuzzgen#4771

Merged
jameysharp merged 1 commit intobytecodealliance:mainfrom
afonso360:fuzz-disable-i128-div
Aug 29, 2022
Merged

cranelift: Disable i128 divs on fuzzgen#4771
jameysharp merged 1 commit intobytecodealliance:mainfrom
afonso360:fuzz-disable-i128-div

Conversation

@afonso360
Copy link
Contributor

Do not merge!

This is going to change the fuzzer input format and invalidate the pending fuzzer issues.

Disables i128 div's from being generated since the x64 backend isn't ready for them yet.

Fixes: #4756
Fixes: #4770
cc: @cfallin @bnjbvr

@bjorn3
Copy link
Contributor

bjorn3 commented Aug 24, 2022

For reference cg_clif uses compiler-builtins intrinsics for 128bit multiplication and division.

@afonso360
Copy link
Contributor Author

I think we already support i128 multiplications (at least the test case is enabled), but I'd still like to implement i128 div/rem and cleanup cg_clif's codegen!

@bjorn3
Copy link
Contributor

bjorn3 commented Aug 24, 2022

Won't 128bit div and rem be rather large? Using compiler-builtins avoids code bloat in those cases.

@afonso360
Copy link
Contributor Author

Well the plan was to do exactly that, lower them as libcalls, or are they not available in all platforms?

@bjorn3
Copy link
Contributor

bjorn3 commented Aug 24, 2022

I see. I believe it is available on all platforms. However LLVM has changed the abi of 128bit compiler builtins on windows before and this may happen again in the future.

@github-actions github-actions bot added the cranelift Issues related to the Cranelift code generator label Aug 24, 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.

When we have the other issues from the current batch of fuzzing sorted out, I approve these changes. Alternatively, if anyone wants to put together an x64 implementation of i128 division, we could merge that immediately since it doesn't change the interpretation of the fuzzer input. I don't think that implementation has to produce good code; as long as it's correct, we can improve its quality later.

@afonso360 afonso360 marked this pull request as ready for review August 28, 2022 14:23
@jameysharp jameysharp merged commit 7663cc1 into bytecodealliance:main Aug 29, 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

3 participants