Conversation
leonardt
commented
Sep 14, 2022
- Promote int-like values in get_slice/set_slice
- Update doc to refer to set_index instead of set_bit
- Support set_slice of length 1 for constant values
* Promote int-like values in get_slice/set_slice * Update doc to refer to set_index instead of set_bit * Support set_slice of length 1 for constant values
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1139 +/- ##
==========================================
+ Coverage 85.00% 85.03% +0.02%
==========================================
Files 154 154
Lines 16368 16375 +7
==========================================
+ Hits 13914 13924 +10
+ Misses 2454 2451 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
|
|
||
| ### set_bit | ||
| ### set_index |
There was a problem hiding this comment.
I would be in favor of renaming the function (publicly, i.e. in the docs) to set_bit. We could deprecate set_index for now with warning (doubt anyone is using it anyway...)
There was a problem hiding this comment.
It was changed to set_index because the pattern is more general than just set_bit (i.e. you can use it on generic arrays, rather than just on Bits), so I think it makes more sense to keep it as set_index for generality
There was a problem hiding this comment.
I see. In that case, why do we need a separate function? I.e. why is it not covered by set slice with width 1?
There was a problem hiding this comment.
Simpler to use when you know you're only interested in one index. For the same reasons in Python we have x[1] versus having to do x[1:2].
There was a problem hiding this comment.
Makes sense -- but I would argue x[1] and x[1:2] is the same interface with different arguments (in fact it literally is the function __getitem__).
Maybe we don't need to do this in this change, but seems reasonable to refactor to have the following interface:
"""
* target: Array[N, T]
* value: Union[Array[M, T], T]
* start: UInt[W]
where M < N and W <= clog2(N)
"""
def set(target: Array, value: Union[Type, Array], start: UInt):
...Now that I think about it there are few new concerns here:
- Should this function work with all
Arrays and not justBits? I kind of assumed it did, but looking at the code closer I'm not certain. - Do we need a
widthargument? Can it just be inferred fromvalue? Ifvalueis an array of the same dimensionality astargetthen it is a slice set; else if it is of dimensionality one less thanvaluethen it is an index set (of course assuming the contained types match up and all). - In the case
startbeing a constant value, should we optimize for the simple wiring (non-dynamic mux) case?
|
Moving this conversation here:
I think my note was unclear. I meant the case when |