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

Native Z3 interface #848

Closed
wants to merge 1 commit into from
Closed

Conversation

manasij7479
Copy link
Collaborator

*.with.overflow encoding seems mysteriously broken.
Other things work, and is about 20-30% faster than our klee+smtlib interface.
Don't merge yet.

@manasij7479
Copy link
Collaborator Author

If any of you want to look:
./souper-check -souper-use-z3 filename.opt

%x:i25 = var
%y:i25 = var
%z = sadd.with.overflow %x, %y
%ov = extractvalue %z, 1
pc %ov 0:i1
%r = extractvalue %z, 0
infer %r
%r2 = addnsw %x, %y
result %r2

@manasij7479
Copy link
Collaborator Author

Another weird bug, documenting here to pick up later

%A:i3 = var
%B:i3 = var
%X:i3 = var
%s = shl 1, %A
%Y = lshr %s, %B
%r = udiv %X, %Y
infer %r
%sub = sub %A, %B
%Y2 = shl 1, %sub
%r2 = udiv %X, %Y2
result %r2

z3 exprs generated:

LHS:(bvudiv %var_X (bvlshr (bvshl #b001 %var_A) %var_B))
RHS:(bvudiv %var_X (bvshl #b001 (bvsub %var_A %var_B)))
(define-fun %var_X () (_ BitVec 3)
  #b011)
(define-fun %var_A () (_ BitVec 3)
  #b101)
(define-fun %var_B () (_ BitVec 3)
  #b100)
  Invalid

@regehr
Copy link
Collaborator

regehr commented Jun 12, 2021

nice!!
should I start to read and give feedback? I'm out hiking Sat but around other than that

@manasij7479
Copy link
Collaborator Author

Another simple looking transform which doesn't verify.

%X:i8 = var
%Y:i8 = var
%c:i1 = var
%sel = select %c, %Y, 0
%r = udiv %X, %sel
infer %r
%r2 = udiv %X, %Y
result %r2

@regehr
Copy link
Collaborator

regehr commented Jun 13, 2021

it might make you feel better to know that the problem with overflow intrinsics isn't related to path conditions, this can't be verified either:

%x:i25 = var
%y:i25 = var
%z = sadd.with.overflow %x, %y
%r = extractvalue %z, 0
infer %r
%r2 = add %x, %y
result %r2

@regehr
Copy link
Collaborator

regehr commented Jun 13, 2021

ok so your add.with.overflow isn't computing the right result when there's overflow.

remember: always simplify the circumstances surrounding a failing test.

%x:i4 = var
%y:i4 = var
%z = sadd.with.overflow 15:i4, 2:i4
%r = extractvalue %z, 0
infer %r
result 1:i4

(this verifies using the old backend and using Alive2, but not using your new one)

@regehr
Copy link
Collaborator

regehr commented Jun 13, 2021

this one does verify

regehr@home:~/souper-manasij$ cat foo.opt
%X:i8 = var
%Y:i8 = var
%c:i1 = var
%sel = select %c, %Y, 0
%r = udiv %X, %sel
infer %r
%r2 = udiv %X, %Y
result %r2
regehr@home:~/souper-manasij$ ./build/souper-check foo.opt -souper-double-check 
; LGTM
regehr@home:~/souper-manasij$ ./build/souper-check foo.opt -souper-double-check -souper-use-z3
PC: #b1
LHS:(bvudiv %var_X (ite (= %var_c #b1) %var_Y #x00))
RHS:(bvudiv %var_X %var_Y)
; LGTM
regehr@home:~/souper-manasij$ 

@regehr
Copy link
Collaborator

regehr commented Jun 13, 2021

so does this one

regehr@home:~/souper-manasij$ cat foo.opt
%A:i3 = var
%B:i3 = var
%X:i3 = var
%s = shl 1, %A
%Y = lshr %s, %B
%r = udiv %X, %Y
infer %r
%sub = sub %A, %B
%Y2 = shl 1, %sub
%r2 = udiv %X, %Y2
result %r2
regehr@home:~/souper-manasij$ ./build/souper-check foo.opt -souper-double-check -souper-use-z3
PC: #b1
LHS:(bvudiv %var_X (bvlshr (bvshl #b001 %var_A) %var_B))
RHS:(bvudiv %var_X (bvshl #b001 (bvsub %var_A %var_B)))
; LGTM
regehr@home:~/souper-manasij$ ./build/souper-check foo.opt -souper-double-check 
; LGTM
regehr@home:~/souper-manasij$ 

@regehr
Copy link
Collaborator

regehr commented Jun 13, 2021

-souper-use-z3 isn't a very informative option since we already have 2 different ways to uses Z3 and you're adding a third, please change this a bit

@manasij7479
Copy link
Collaborator Author

concat order was wrong, everything seems to be working now.
This isn't integrated with synthesis yet, but all the solver checks pass.

@manasij7479 manasij7479 marked this pull request as ready for review June 13, 2021 23:54
@regehr
Copy link
Collaborator

regehr commented Jun 14, 2021

ok, nice work!

I want a test case for timeouts using this API. do like a 1024 bit division or something like that where it has no chance whatsoever of completing.

have you tried turning off the KLEE code entirely? does everything still work in that case?

}
bool verify(Inst *RHS, Inst *RHSAssumptions = nullptr) {
Translate(RHS);
if (RHSAssumptions) AddConstraint(RHSAssumptions);
Copy link
Collaborator

Choose a reason for hiding this comment

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

put the "then" part on another line please

@regehr
Copy link
Collaborator

regehr commented Jun 14, 2021

so @manasij7479 do you want to land this and then do models in a separate pr? if so, that's fine

I want you to support (and test) Z3 timeouts in this PR though

}

bool Translate(souper::Inst *I) {
if (!I) return true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

on a new line


}

std::string Name = "";
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove unnecessary initializer

case souper::Inst::Var: {
Put(I, ctx.bv_const(Name.c_str(), W));
auto DFC = getDataflowConditions(I, IC);
if (DFC) AddConstraint(DFC);
Copy link
Collaborator

Choose a reason for hiding this comment

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

put the then part on a new line

@regehr
Copy link
Collaborator

regehr commented Jun 14, 2021

@manasij7479 can you explain what the changes to the alive driver are doing?


namespace souper {
bool isTransformationValidZ3(souper::Inst *LHS, souper::Inst *RHS,
const std::vector<InstMapping> &PCs,
Copy link
Collaborator

Choose a reason for hiding this comment

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

fix indentation

@manasij7479
Copy link
Collaborator Author

AliveDriver changes are just splitting out reusable code into a separate TU.

@manasij7479
Copy link
Collaborator Author

I want you to support (and test) Z3 timeouts in this PR though

Will do.

@manasij7479
Copy link
Collaborator Author

Implemented but not yet tested support for timeouts.
Even for larger bitwidths, z3 tactics seem to be taking care of solving things fast.
Have to find a weird valid transformation which escapes simplification.

@fvrmatteo
Copy link

@manasij7479 the "weird valid transformation which escapes simplification" could be here: #738. Most likely the timeout will kick in before the synthesis candidate is verified as correct. I hope this helps and is not out of scope.

@regehr
Copy link
Collaborator

regehr commented Jun 21, 2021

very wide division by non-constant should do it

@manasij7479
Copy link
Collaborator Author

See the newer PR

@manasij7479 manasij7479 closed this Sep 8, 2021
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