Skip to content

SVA: implement ranged s_eventually operator #691

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

Merged
merged 1 commit into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion regression/verilog/SVA/s_eventually2.desc
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
CORE
s_eventually2.sv
--module main --bound 20
^EXIT=0$
^\[main\.p0\] always s_eventually main.reset \|\| main\.counter == 10: PROVED up to bound 20$
^\[main\.p1\] always \(s_eventually \[0:2\] main.reset \|\| main\.counter == 10\): REFUTED$
^EXIT=10$
^SIGNAL=0$
--
^warning: ignoring
3 changes: 3 additions & 0 deletions regression/verilog/SVA/s_eventually2.sv
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,7 @@ module main(input clk, input reset);
// expected to pass for any bound
p0: assert property (s_eventually (reset || counter == 10));

// expected to fail, owing to range
p1: assert property (s_eventually[0:2] (reset || counter == 10));

endmodule
1 change: 1 addition & 0 deletions src/hw_cbmc_irep_ids.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ IREP_ID_ONE(sva_nexttime)
IREP_ID_ONE(sva_s_nexttime)
IREP_ID_ONE(sva_eventually)
IREP_ID_ONE(sva_s_eventually)
IREP_ID_ONE(sva_ranged_s_eventually)
IREP_ID_ONE(sva_until)
IREP_ID_ONE(sva_s_until)
IREP_ID_ONE(sva_until_with)
Expand Down
40 changes: 40 additions & 0 deletions src/trans-word-level/instantiate_word_level.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Author: Daniel Kroening, [email protected]
#include <util/ebmc_util.h>
#include <util/expr_util.h>

#include <ebmc/ebmc_error.h>
#include <temporal-logic/temporal_expr.h>
#include <temporal-logic/temporal_logic.h>
#include <verilog/sva_expr.h>
Expand Down Expand Up @@ -428,6 +429,45 @@ wl_instantiatet::instantiate_rec(exprt expr, const mp_integer &t) const
DATA_INVARIANT(no_timeframes != 0, "must have timeframe");
return {no_timeframes - 1, conjunction(conjuncts)};
}
else if(expr.id() == ID_sva_ranged_s_eventually)
{
Comment on lines +432 to +433
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not that this change makes the decisive difference, but: instantiate_rec has become very very long and could do with breaking into smaller functions. Perhaps this opportunity could be taken to at least move this case to a function of its own?

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do. What bugs me most is the duplication between instantiate_word_level.cpp and property.cpp.

auto &phi = to_sva_ranged_s_eventually_expr(expr).op();
auto &lower = to_sva_ranged_s_eventually_expr(expr).lower();
auto &upper = to_sva_ranged_s_eventually_expr(expr).upper();

auto from_opt = numeric_cast<mp_integer>(lower);
if(!from_opt.has_value())
throw ebmc_errort() << "failed to convert SVA s_eventually from index";

auto from = t + std::max(mp_integer{0}, *from_opt);

mp_integer to;

if(upper.id() == ID_infinity)
{
throw ebmc_errort()
<< "failed to convert SVA s_eventually to index (infinity)";
}
else
Comment on lines +444 to +451
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this check could be done earlier so that to could use RAII and also its handling would be symmetric to that of from.

Copy link
Member Author

Choose a reason for hiding this comment

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

RAII won't do it -- it's perfectly fine to initialize with infinity. What's missing here is the comment "the normalizer has rewritten this". I'll implement the case, since I want to rely less on the normalizer doing stuff. It's just one line.

{
auto to_opt = numeric_cast<mp_integer>(upper);
if(!to_opt.has_value())
throw ebmc_errort() << "failed to convert SVA s_eventually to index";
to = std::min(t + *to_opt, no_timeframes - 1);
}

exprt::operandst disjuncts;
mp_integer time = 0;

for(mp_integer c = from; c <= to; ++c)
{
auto tmp = instantiate_property(phi, c, no_timeframes, ns);
time = std::max(time, tmp.first);
disjuncts.push_back(tmp.second);
}

return {time, disjunction(disjuncts)};
}
else if(expr.id()==ID_sva_until ||
expr.id()==ID_sva_s_until)
{
Expand Down
5 changes: 5 additions & 0 deletions src/trans-word-level/obligations.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ class obligationst
public:
obligationst() = default;

explicit obligationst(const mp_integer &t, const exprt &expr)
{
add(t, expr);
}

explicit obligationst(const std::pair<mp_integer, exprt> &pair)
{
add(pair.first, pair.second);
Expand Down
5 changes: 5 additions & 0 deletions src/verilog/expr2verilog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1521,6 +1521,11 @@ expr2verilogt::resultt expr2verilogt::convert_rec(const exprt &src)
return precedence = verilog_precedencet::MIN,
convert_sva_unary("s_eventually", to_sva_s_eventually_expr(src));

else if(src.id() == ID_sva_ranged_s_eventually)
return precedence = verilog_precedencet::MIN,
convert_sva_ranged_predicate(
"s_eventually", to_sva_ranged_s_eventually_expr(src));

else if(src.id()==ID_sva_until)
return precedence = verilog_precedencet::MIN,
convert_sva_binary("until", to_sva_until_expr(src));
Expand Down
2 changes: 1 addition & 1 deletion src/verilog/parser.y
Original file line number Diff line number Diff line change
Expand Up @@ -2197,7 +2197,7 @@ property_expr_proper:
| "eventually" '[' constant_range ']' property_expr %prec "eventually"
{ init($$, ID_sva_eventually); swapop($$, $3); mto($$, $5); }
| "s_eventually" '[' cycle_delay_const_range_expression ']' property_expr %prec "s_eventually"
{ init($$, ID_sva_s_eventually); swapop($$, $3); mto($$, $5); }
{ init($$, ID_sva_ranged_s_eventually); swapop($$, $3); mto($$, $5); }
| property_expr "until" property_expr
{ init($$, "sva_until"); mto($$, $1); mto($$, $3); }
| property_expr "s_until" property_expr
Expand Down
29 changes: 29 additions & 0 deletions src/verilog/sva_expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,35 @@ static inline sva_s_eventually_exprt &to_sva_s_eventually_expr(exprt &expr)
return static_cast<sva_s_eventually_exprt &>(expr);
}

class sva_ranged_s_eventually_exprt : public sva_ranged_predicate_exprt
{
public:
explicit sva_ranged_s_eventually_exprt(exprt lower, exprt upper, exprt op)
: sva_ranged_predicate_exprt(
ID_sva_s_eventually,
std::move(lower),
std::move(upper),
std::move(op))
{
}
};

static inline const sva_ranged_s_eventually_exprt &
to_sva_ranged_s_eventually_expr(const exprt &expr)
{
PRECONDITION(expr.id() == ID_sva_ranged_s_eventually);
sva_ranged_s_eventually_exprt::check(expr, validation_modet::INVARIANT);
return static_cast<const sva_ranged_s_eventually_exprt &>(expr);
}

static inline sva_ranged_s_eventually_exprt &
to_sva_ranged_s_eventually_expr(exprt &expr)
{
PRECONDITION(expr.id() == ID_sva_ranged_s_eventually);
sva_ranged_s_eventually_exprt::check(expr, validation_modet::INVARIANT);
return static_cast<sva_ranged_s_eventually_exprt &>(expr);
}

class sva_always_exprt : public unary_predicate_exprt
{
public:
Expand Down
4 changes: 2 additions & 2 deletions src/verilog/verilog_typecheck_expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3108,8 +3108,8 @@ exprt verilog_typecheck_exprt::convert_trinary_expr(ternary_exprt expr)
return std::move(expr);
}
else if(
expr.id() == ID_sva_eventually || expr.id() == ID_sva_ranged_always ||
expr.id() == ID_sva_s_always)
expr.id() == ID_sva_eventually || expr.id() == ID_sva_ranged_s_eventually ||
expr.id() == ID_sva_s_always || expr.id() == ID_sva_ranged_always)
{
auto lower = convert_integer_constant_expression(expr.op0());

Expand Down