-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Added rewriting distinct with bitvectors to false if bit-size is too low #5956
Conversation
src/ast/rewriter/bv_rewriter.cpp
Outdated
br_status bv_rewriter::mk_bvsmul_no_overflow(unsigned num, expr * const * args, bool is_overflow, expr_ref & result) { | ||
br_status bv_rewriter::mk_distinct(unsigned num_args, expr * const * args, expr_ref & result) { | ||
if (num_args == 0) { | ||
return BR_FAILED; |
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.
result = true
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.
and if num_args = 1 then result = false.
OTOH, these rewrites are already handled by bool_rewriter
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.
Do you mean result = true for num_args = 1?
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, right, true for both 0 and 1 arguments.
src/ast/rewriter/bv_rewriter.cpp
Outdated
exact &= (num_args % 2) == 0; | ||
num_args /= 2; | ||
sz--; | ||
} while (num_args > 1 && sz > 0); |
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.
convention: while on the next line after }.
I know this is contrary to some automatic formatting modes
src/ast/rewriter/th_rewriter.cpp
Outdated
to_app(args[0])->get_family_id() == m_seq_rw.get_fid()) { | ||
st = m_seq_rw.mk_eq_core(args[0], args[1], result); | ||
if (st != BR_FAILED) | ||
return st; | ||
} | ||
if (k == OP_DISTINCT) { | ||
SASSERT(num > 0); |
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.
just add it to the if statement
if (k == OP_DISTINCT && num > 0)
and for that matter m_bv_rw.is_bv_sort(args[0]->get_sort())
return BR_DONE; | ||
} | ||
unsigned sz = get_bv_size(args[0]); | ||
// check if num_args > 2^sz |
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.
this may be clever code, but not easy to read.
Here is something simpler:
if (sz >= 32)
return BR_FAILED;
if (num_args <= 1u << sz)
return BR_FAILED;
result = m().mk_false();
return BR_DONE;
src/ast/rewriter/th_rewriter.cpp
Outdated
to_app(args[0])->get_family_id() == m_seq_rw.get_fid()) { | ||
st = m_seq_rw.mk_eq_core(args[0], args[1], result); | ||
if (st != BR_FAILED) | ||
return st; | ||
} | ||
if (k == OP_DISTINCT && num > 0) { |
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.
Why two nested ifs?
No description provided.