Skip to content

Implement __builtin_ctz{,l,ll} via count_trailing_zeros_exprt #5881

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
May 1, 2021

Conversation

tautschnig
Copy link
Collaborator

@tautschnig tautschnig commented Mar 2, 2021

Marked as Draft as it depends on #5912.

__builtin_ctz returns the number of trailing zeros. Just like ffs and clz, introduce a new bitvector expression.

  • Each commit message has a non-empty body, explaining why the change was made.
  • Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
  • n/a The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
  • Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
  • n/a My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • n/a White-space or formatting changes outside the feature-related changed lines are in commits of their own.

@tautschnig tautschnig self-assigned this Mar 2, 2021
@tautschnig tautschnig force-pushed the ctz-expression branch 2 times, most recently from e7169de to e0ddd62 Compare March 2, 2021 21:35
@codecov
Copy link

codecov bot commented Mar 2, 2021

Codecov Report

Merging #5881 (730c461) into develop (0e34169) will increase coverage by 0.00%.
The diff coverage is 42.76%.

Impacted file tree graph

@@            Coverage Diff            @@
##           develop    #5881    +/-   ##
=========================================
  Coverage    74.25%   74.25%            
=========================================
  Files         1445     1445            
  Lines       157479   157635   +156     
=========================================
+ Hits        116933   117059   +126     
- Misses       40546    40576    +30     
Impacted Files Coverage Δ
src/ansi-c/expr2c.cpp 58.48% <ø> (ø)
src/util/format_expr.cpp 84.87% <0.00%> (-0.79%) ⬇️
src/util/pointer_expr.cpp 72.82% <0.00%> (-18.96%) ⬇️
src/util/pointer_expr.h 77.34% <0.00%> (-22.66%) ⬇️
src/util/simplify_expr_class.h 100.00% <ø> (ø)
...riable-sensitivity/abstract_object/index_range.cpp 100.00% <ø> (ø)
src/solvers/flattening/bv_pointers.cpp 81.26% <13.04%> (-3.36%) ⬇️
src/ansi-c/c_typecheck_expr.cpp 75.17% <75.00%> (-0.01%) ⬇️
src/util/simplify_expr.cpp 76.25% <76.92%> (-8.24%) ⬇️
src/util/bitvector_expr.h 93.07% <90.00%> (-5.97%) ⬇️
... and 49 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1c54a96...730c461. Read the comment docs.

@martin-cs
Copy link
Collaborator

How does this relate to #5879 ?

@tautschnig
Copy link
Collaborator Author

How does this relate to #5879 ?

There is a code overlap in goto_checkt that in principle I could resolve to make them entirely independent, but I'm not too keen on first effectively creating a future merge conflict and later resolving that.

@tautschnig tautschnig changed the title Implement __builtin_ctz{,l,ll} via count_trailing_zeros_exprt Implement __builtin_ctz{,l,ll} via count_trailing_zeros_exprt [depends-on: #5912] Mar 18, 2021
@tautschnig tautschnig marked this pull request as ready for review March 25, 2021 21:20
@tautschnig tautschnig changed the title Implement __builtin_ctz{,l,ll} via count_trailing_zeros_exprt [depends-on: #5912] Implement __builtin_ctz{,l,ll} via count_trailing_zeros_exprt Mar 25, 2021
@tautschnig tautschnig removed their assignment Mar 25, 2021
simplify_exprt::resultt<>
simplify_exprt::simplify_ctz(const count_trailing_zeros_exprt &expr)
{
const auto const_bits_opt = expr2bits(
Copy link
Member

Choose a reason for hiding this comment

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

Does it make sense to use the lowering in the simplifier to avoid the duplicate implementation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We could, but it's probably more expensive. Whether that's a practical concern, however, is a different question. I suppose ctz will be fairly rare.

__builtin_ctz returns the number of trailing zeros. Just like ffs and
clz, introduce a new bitvector expression.
@tautschnig tautschnig merged commit cc30590 into diffblue:develop May 1, 2021
@tautschnig tautschnig deleted the ctz-expression branch May 1, 2021 22:13
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.

4 participants