-
Notifications
You must be signed in to change notification settings - Fork 33
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
feat(BV, CP): Add bitlist propagators for add/sub #1151
Conversation
ddb3a75
to
b424e89
Compare
I read carefully your comments and the proofs seem okay but my concern about these comments is that I have no intuition at all after reading the proof. I basically checked them step by step. I believe we can do better and actually I found a simpler but a bit more abstract proof. I keep your definition of carry position. I also extract your first lemma but I suggest an alternative proof:
Proof: We proceed by induction on the carry position Now, we state the main result:
Before proving this lemma, let us introduce few notations. For bitvectors Now, remark that any value Let us prove the equivalence:
|
src/lib/reasoners/bitlist.ml
Outdated
disagree (i.e. have different values for that bit). Let us furthermore | ||
assume that [i] is known in both [a] and [b] (i.e. it is set in both | ||
[a.bits_unk] and [b.bits_unk]). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought that bits_unk
is the set of unknown bits? Do you mean that i
is not active in a.bits_unk
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I meant that it is cleared in both. Will fix.
src/lib/reasoners/bitlist.ml
Outdated
@@ -397,3 +397,70 @@ let bvlshr ~size:sz a b = | |||
extract unknown 0 sz | |||
| _ | exception Z.Overflow -> | |||
constant Z.zero (explanation b) | |||
|
|||
let add a b = | |||
(* A binary addition [x + y] has a carry at bit position [i] iff the addition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should start this comment by explaining what we are proving, otherwise we have to read all the comment before discovering that it is a proof of correction.
src/lib/reasoners/bitlist.ml
Outdated
Hence, unknown bits in [a + b] are the bits that are either unknown in | ||
[a], unknown in [b], or differ in [a.bits_set + b.bits_set] and in | ||
[a.bits_set + a.bits_unk + b.bits_set + b.bits_unk]. *) | ||
let x = Z.add a.bits_set b.bits_set in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is confusing to use x
here because x
denotes a completely different thing in the comment above.
src/lib/reasoners/bitlist.ml
Outdated
|
||
Recalling [(~b).bits_set = ~(b.bits_set | b.bits_unk)], we get the formula | ||
below. *) | ||
let x = Z.sub a.bits_set b.bits_set in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same remark.
This patch adds propagators on the bitlist domain for addition and subtraction. These propagators are able to compute low bits independently of high bits; in particular, we now know that the sum of two even (or two odd) numbers is even. If there is a bit pattern that prevents carry propagation (e.g. two [0]s for addition), we are also able to compute the following bits precisely. Note that we do not currently decompose addition/subtraction according to these propagators -- for instance, we do not know that `(bvadd (concat x #b0) (concat y #b0))` is `(concat (bvadd x y) #b0)`.
@Halbaroth Rewrote the proof taking in consideration your remarks. I tried to avoid induction since I don't think it is necessary here if we think about integers. Let me know if this is better. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks :)
Please don't merge before I finish #1169
* feat(CP): Add bitlist propagators for add/sub This patch adds propagators on the bitlist domain for addition and subtraction. These propagators are able to compute low bits independently of high bits; in particular, we now know that the sum of two even (or two odd) numbers is even. If there is a bit pattern that prevents carry propagation (e.g. two [0]s for addition), we are also able to compute the following bits precisely. Note that we do not currently decompose addition/subtraction according to these propagators -- for instance, we do not know that `(bvadd (concat x #b0) (concat y #b0))` is `(concat (bvadd x y) #b0)`. * Better proof
This patch adds propagators on the bitlist domain for addition and
subtraction. These propagators are able to compute low bits
independently of high bits; in particular, we now know that the sum of
two even (or two odd) numbers is even.
If there is a bit pattern that prevents carry propagation (e.g. two [0]s
for addition), we are also able to compute the following bits precisely.
Note that we do not currently decompose addition/subtraction according
to these propagators -- for instance, we do not know that
(bvadd (concat x #b0) (concat y #b0))
is(concat (bvadd x y) #b0)
.Note: depends on #1058, #1083, #1084, #1085, and #1144. Only the latest commit with title "feat(CP): Add bitlist propagators for add/sub" is included in this PR.