Simplify multiple-of-element size access to arrays #8627
Conversation
Array operations may fall back to byte_extract or byte_update expressions in parts of the code base. Simplify this to index or WITH expressions, respectively, when the offset is known to be a multiple of the array-element size.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #8627 +/- ##
========================================
Coverage 80.37% 80.37%
========================================
Files 1686 1686
Lines 206872 206968 +96
Branches 73 73
========================================
+ Hits 166265 166360 +95
- Misses 40607 40608 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @@ -687,7 +687,58 @@ std::optional<exprt> get_subexpression_at_offset( | |||
| const auto offset_bytes = numeric_cast<mp_integer>(offset); | |||
|
|
|||
| if(!offset_bytes.has_value()) | |||
There was a problem hiding this comment.
does has_value mean "literal value" ?
| if( | ||
| !target_size_bits.has_value() || !elem_size_bits.has_value() || | ||
| *elem_size_bits <= 0 || | ||
| *elem_size_bits % config.ansi_c.char_width != 0 || |
There was a problem hiding this comment.
Does this still allow the optimization to apply on arrays of type T where alignof(T) > 8 ?
| return {}; | ||
| } | ||
|
|
||
| if( |
There was a problem hiding this comment.
bail if expression is not of the form K * i
| @@ -1870,7 +1872,7 @@ simplify_exprt::simplify_byte_extract(const byte_extract_exprt &expr) | |||
| expr2bits(expr.op(), expr.id() == ID_byte_extract_little_endian, ns); | |||
|
|
|||
| if( | |||
| bits.has_value() && | |||
| offset.has_value() && bits.has_value() && | |||
There was a problem hiding this comment.
What is this doing exactly ? seems like it restricts the optimization to a subset of cases ?
| @@ -1986,7 +1988,9 @@ simplify_exprt::simplify_byte_extract(const byte_extract_exprt &expr) | |||
| const array_typet &array_type = to_array_type(expr.op().type()); | |||
| const auto &element_bit_width = | |||
| pointer_offset_bits(array_type.element_type(), ns); | |||
| if(element_bit_width.has_value() && *element_bit_width > 0) | |||
| if( | |||
There was a problem hiding this comment.
again it seems like the optimization would apply less often ?
| return changed(simplify_rec(result_expr)); | ||
| } | ||
| else if( | ||
| offset.id() == ID_mult && offset.operands().size() == 2 && |
There was a problem hiding this comment.
this optimizes the encoding of an array update, and the very similar code above optimized a read ?
| @@ -687,7 +687,58 @@ std::optional<exprt> get_subexpression_at_offset( | |||
| const auto offset_bytes = numeric_cast<mp_integer>(offset); | |||
|
|
|||
| if(!offset_bytes.has_value()) | |||
| return {}; | |||
| { | |||
There was a problem hiding this comment.
Is this the array read encoding ?
|
Is there a way to share some of the pattern matching conditions between the array-read and array-write cases ? |
|
Seems like we should also be able to pattern match an access to a member of a struct in an array of structs like a[i].m, the offset would be of the form i*sizeof(T) + offset(T,m) and turn that into a functional update as well ? |
Array operations may fall back to byte_extract or byte_update expressions in parts of the code base. Simplify this to index or WITH expressions, respectively, when the offset is known to be a multiple of the array-element size.