Skip to content

Verilog: ranged always properties #432

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
Apr 15, 2024
Merged

Verilog: ranged always properties #432

merged 1 commit into from
Apr 15, 2024

Conversation

kroening
Copy link
Member

@kroening kroening commented Apr 2, 2024

This adds support for always property expressions with a given range.

@kroening kroening force-pushed the sva_ranged_always branch 3 times, most recently from 087a57a to aa3d47d Compare April 3, 2024 18:48
@kroening kroening marked this pull request as ready for review April 3, 2024 19:09
@kroening kroening force-pushed the sva_ranged_always branch from aa3d47d to 46f69a6 Compare April 3, 2024 19:09
Comment on lines 82 to 83
mp_integer current,
mp_integer no_timeframes,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Given that mp_integer is non-trivial, how about passing those as const references?

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Comment on lines 131 to 153
mp_integer from, to;

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

from = *from_opt;

if(range.op1().id() == ID_infinity)
{
to = no_timeframes - 1;
}
else
{
auto to_opt = numeric_cast<mp_integer>(range.op1());
if(!to_opt.has_value())
throw ebmc_errort() << "failed to convert SVA always to index";
to = *to_opt;
}

for(mp_integer c = from; c <= to; ++c)
{
if(c >= 0 && c < no_timeframes)
Copy link
Collaborator

Choose a reason for hiding this comment

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

A couple of (related) ideas:

  • No need to declare from before it can be initialised.
  • Taking this one step further, how about: mp_integer from = std::max(0, *from_opt);
  • Use to = std::min(no_timeframes - 1, *to_opt);
    These together will avoid unnecessary increments of c and permit removing the if inside this for loop.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

const exprt &property_expr,
decision_proceduret &solver,
std::size_t no_timeframes,
mp_integer no_timeframes,
Copy link
Collaborator

Choose a reason for hiding this comment

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

As above: const mp_integer &?

Copy link
Member Author

Choose a reason for hiding this comment

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

done

class sva_ranged_always_exprt : public binary_predicate_exprt
{
public:
explicit sva_ranged_always_exprt(binary_exprt range, exprt op)
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need for explicit

class sva_s_always_exprt : public binary_predicate_exprt
{
public:
explicit sva_s_always_exprt(binary_exprt range, exprt op)
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need for explicit

Comment on lines +1595 to +1604
#if 0
else if(src_type.id() == ID_natural)
{
if(dest_type.id()==ID_integer)
{
expr = typecast_exprt{expr, dest_type};
return;
}
}
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could a comment please be included to explain why this is defined away?

Copy link
Member Author

Choose a reason for hiding this comment

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

removed the #if 0

This adds support for always property expressions with a given range and for
s_always properties.
@kroening kroening force-pushed the sva_ranged_always branch from 46f69a6 to f664e6b Compare April 15, 2024 15:27
@kroening kroening merged commit 4b56ef0 into main Apr 15, 2024
4 checks passed
@kroening kroening deleted the sva_ranged_always branch April 15, 2024 15:58
tautschnig added a commit that referenced this pull request Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants