Skip to content

Commit

Permalink
#6100 - two perf fixes
Browse files Browse the repository at this point in the history
remaining perf bug is dealing with very large bit-widths. mod 2^n should be computed natively based on n instead of 2^n because we pre-populate an array with all values up to n. Suppose n is 10000, the array has size 10000.
  • Loading branch information
NikolajBjorner committed Jun 21, 2022
1 parent f24c5ca commit 202ce1e
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 4 deletions.
7 changes: 5 additions & 2 deletions src/ast/bv_decl_plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,8 @@ func_decl * bv_decl_plugin::mk_num_decl(unsigned num_parameters, parameter const
// This cannot be enforced now, since some Z3 modules try to generate these invalid numerals.
// After SMT-COMP, I should find all offending modules.
// For now, I will just simplify the numeral here.
parameter p0(mod(parameters[0].get_rational(), rational::power_of_two(bv_size)));
rational v = parameters[0].get_rational();
parameter p0(mod(v, rational::power_of_two(bv_size)));
parameter ps[2] = { std::move(p0), parameters[1] };
sort * bv = get_bv_sort(bv_size);
return m_manager->mk_const_decl(m_bv_sym, bv, func_decl_info(m_family_id, OP_BV_NUM, num_parameters, ps));
Expand Down Expand Up @@ -645,11 +646,13 @@ void bv_decl_plugin::get_offset_term(app * a, expr * & t, rational & offset) con
}
else {
t = a;
offset = rational(0);
offset.reset();
}
}

bool bv_decl_plugin::are_distinct(app * a, app * b) const {
if (is_value(a) && is_value(b))
return a != b;
#if 1
// Check for a + k1 != a + k2 when k1 != k2
rational a_offset;
Expand Down
13 changes: 11 additions & 2 deletions src/ast/rewriter/array_rewriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ Module Name:
#include "ast/ast_ll_pp.h"
#include "ast/rewriter/var_subst.h"
#include "params/array_rewriter_params.hpp"
#include "util/util.h"

void array_rewriter::updt_params(params_ref const & _p) {
array_rewriter_params p(_p);
Expand Down Expand Up @@ -161,7 +162,8 @@ br_status array_rewriter::mk_store_core(unsigned num_args, expr * const * args,

return BR_FAILED;
}



br_status array_rewriter::mk_select_core(unsigned num_args, expr * const * args, expr_ref & result) {
SASSERT(num_args >= 2);
if (m_util.is_store(args[0])) {
Expand All @@ -172,9 +174,16 @@ br_status array_rewriter::mk_select_core(unsigned num_args, expr * const * args,
result = to_app(args[0])->get_arg(num_args);
return BR_DONE;
case l_false: {
expr* arg0 = args[0];
expr* arg1 = to_app(arg0)->get_arg(0);
while (m_util.is_store(arg1) && compare_args<true>(num_args-1, args + 1, to_app(arg0)->get_args() + 1) == l_false) {
arg0 = arg1;
arg1 = to_app(arg0)->get_arg(0);
}

// select(store(a, I, v), J) --> select(a, J) if I != J
ptr_buffer<expr> new_args;
new_args.push_back(to_app(args[0])->get_arg(0));
new_args.push_back(arg1);
new_args.append(num_args-1, args+1);
result = m().mk_app(get_fid(), OP_SELECT, num_args, new_args.data());
return BR_REWRITE1;
Expand Down

3 comments on commit 202ce1e

@nunoplopes
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm seeing a lot of regressions with this patch. Tests are now failing with smt tactic failed to show goal to be sat/unsat (incomplete quantifiers)

@nunoplopes
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's the array rewriter bit that is at fault.

@nunoplopes
Copy link
Collaborator

@nunoplopes nunoplopes commented on 202ce1e Jun 21, 2022

Choose a reason for hiding this comment

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

Bug fixed in: 41deed5

The previous code was advancing the store chain one too much. This was happening:

SELECT: (store (store (store (_ as-array k!39) #x00 #b000000000) #x01 #b000000000) #x02 #b000000000), #x00
NEW ARGS: (_ as-array k!39), #x00

The fixed code produces:

NEW ARGS: (store (_ as-array k!39) #x00 #b000000000), #x00

Please sign in to comment.