Skip to content
This repository has been archived by the owner on Mar 16, 2024. It is now read-only.

Support for Bit Vectors #13

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

Conversation

adaminsky
Copy link
Contributor

No description provided.

@SaswatPadhi
Copy link
Owner

Oh I see the issue with of_string for BitVec type. PR #11 is introducing a of_sexp function that would solve this.

src/Value.ml Outdated Show resolved Hide resolved
+ The TVAR type stores the string representation of the bitvector's
size. Two bitvectors will unify if they have the same size.
+ More operations on bitvectors were added to BitVecComponents.ml and to
Bitarray.ml.
@adaminsky
Copy link
Contributor Author

In order to run benchmarks I have to remove line 9 of Simulator.ml. Also, I think the way I tried to parameterize the bit vector type is probably not that good since I use TVAR to store information for a string instead of another type. Maybe I should create another type which just parameterizes the length of a bit vector.

@SaswatPadhi
Copy link
Owner

Thanks, let me take a look at your current set of changes and then I can provide better feedback.

Copy link
Owner

@SaswatPadhi SaswatPadhi left a comment

Choose a reason for hiding this comment

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

Thanks Adam.

The components file and Bitarray file look good. I have left some comments on the definitions of types and values. The main issue is that we are doing unnecessary Int <-> String conversions for the length (which is also expensive computationally).

I will take another pass once that's fixed.

src/Value.ml Outdated Show resolved Hide resolved
src/Value.ml Outdated Show resolved Hide resolved
src/Value.ml Outdated Show resolved Hide resolved
src/Value.ml Outdated Show resolved Hide resolved
src/Value.ml Outdated Show resolved Hide resolved
src/Type.ml Outdated Show resolved Hide resolved
src/TestGen.ml Outdated Show resolved Hide resolved
… bit

The unification routines try to match a BitVec of negative size to one of
positive size. The negative size is treated as a variable, so all BitVecs of
size -1 will be matched to the same length BitVec. There is still int to string
conversions because the length is parsed as a string and the unification
routines use an environment, env, of type (string * t).
@SaswatPadhi
Copy link
Owner

Hi Adam,

Thanks for all these changes. Please let me know when you want me to take another look at your changes.

@adaminsky
Copy link
Contributor Author

Hi Saswat,
I'm ready for you to take another look at the changes.
Thank you

@adaminsky
Copy link
Contributor Author

I ran into the same problem that you had here with core_extended. Did you ever find a solution to it? It seems like the issue is that the dune file for the bitarray module does not have the (public_name ...) field so the library is not actually being installed by dune. I checked with the Docker container and all the other submodules defined in core_extended are installed by dune except for core_extended.bitarray, and the base core_extended library does not exist. It seems like this could be a bug with the way core_extended is built. I got it to work by adding (public_name core_extended.bitarray) to the bitarray dune file and rebuilding core_extended.

@SaswatPadhi
Copy link
Owner

@adaminsky You're right. I just tried it again and there doesn't seem to be an easy way of using bitarray even though it's packaged within core_extended. I noticed that it was included in core_extended again, but hadn't tried using it.

I would raise another bug report, but for now I think you should move all your additions to some other files (BitArrayExt.ml?) and restore the original BitArray.ml so that when the bug is finally resolved, we can simply remove our copy of BitArray.ml and use the library copy of it.

Bitarray.ml is taken from v0.14.0 of core_extended and included into the
extended BitarrayExt module.

A bitvector benchmark is included adapted from
[cegar2.sl](https://github.com/SyGuS-Org/benchmarks/blob/master/lib-nonconforming/Inv_Track/cegist-cav18/cegar2.sl)
in the SyGuS benchmarks repository. This benchmark currently passes.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants