Skip to content

Known Bits Domain #23

Open
sneha-zazen wants to merge 13 commits intoagle:mainfrom
sneha-zazen:main
Open

Known Bits Domain #23
sneha-zazen wants to merge 13 commits intoagle:mainfrom
sneha-zazen:main

Conversation

@sneha-zazen
Copy link

This domain of tnums (tristate numbers) is used to reason about the bitwise uncertainty in program values.

Copy link
Owner

@agle agle left a comment

Choose a reason for hiding this comment

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

Good work, its probably a good idea to also check in your documentation pdf under /docs as well

if Bitvec.is_nonzero bm then Top
else tnum (Bitvec.ashr av bv) (Bitvec.shl am bv))

let mul =
Copy link
Owner

Choose a reason for hiding this comment

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

Worth commenting which version of mul you implemented and why

Copy link
Author

Choose a reason for hiding this comment

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

the algorithm was tweaked to be written in ocaml so it has recursive calls and cases that follows final version of mul as given in the paper but it takes inspiration from other implementations as well
i have explained it more in the comments but lmk if i should change anything there

let bitLSHR =
bind2 (fun (av, am) (bv, bm) ->
if Bitvec.is_nonzero bm then Top
else tnum (Bitvec.lshr av bv) (Bitvec.shl am bv))
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the mask be logically shifted right here? (and same for ashr)

I suspect that the property test didn't catch this because it occurs in an edge case for when bm is zero

Copy link
Author

Choose a reason for hiding this comment

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

omg you are so right sorry gng mb

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