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

Floating Point Cleanup #48

Merged
merged 3 commits into from
Feb 23, 2024
Merged

Floating Point Cleanup #48

merged 3 commits into from
Feb 23, 2024

Conversation

ncough
Copy link
Collaborator

@ncough ncough commented Feb 22, 2024

A series of improvements to improve the quality of floating point results. Mostly the conversion of enums to bit vectors and some cleanup around trivial case statements. Notably reduces coverage decompilation failures to only those instructions that refer to FPToFixedJS, due to its tuple return.

- Remove throws from generated code
- Block inlining on FPRecipEstimate
- Convert enums to bvs as a post-transform
- Alter semantics to permit int/enum comparison, improving coverage testing results
Match if (x = constant) then r := constant else ... patterns
and attempt to replace with r := f x for simple mappings between
constant pairs.
@katrinafyi
Copy link
Member

A small question about the zext, from the example in #46. Is the extend needed if it's only going to be used by cvt_bits_uint?

In general, I think the int to bits convention is to treat bit vectors as signed. Is this why?

@l-kent
Copy link

l-kent commented Feb 22, 2024

I'd expect in that example it's because the FPRounding type is an enum with 6 possible values so to represent it as a bitvector you need 3 bits, but in that example it's being converted from a 2-bit value which corresponds to one of the first four enum values.

@katrinafyi
Copy link
Member

How interesting!

@ncough
Copy link
Collaborator Author

ncough commented Feb 22, 2024

Yeah, the extend is due to the size of FPRounding type. The goal was to have consistent bit vector widths for arguments to FPConvert, even if they are via cvt_bits_uint, as a convenience to parsers if they chose to simply ignore the int conversion.

@ncough ncough merged commit 064c257 into partial_eval Feb 23, 2024
1 check passed
@ncough ncough deleted the enums branch March 18, 2024 22:18
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