Feat/dla vp overflow fix #69
Conversation
vp/devel/python_peripherals/DLA.py
Outdated
| @@ -318,7 +295,7 @@ def cast_long_to_signed_byte(value): | |||
| Returns: | |||
| byte -- Int Signed value in range -128..127 | |||
| """ | |||
| assert(0 <= value <= 255) | |||
| assert(0 <= value <= 255), "Assert failed! Value doesn't fit byte" | |||
There was a problem hiding this comment.
Might be better expressed as assert(0 <= value <= 0xff)
vp/devel/python_peripherals/DLA.py
Outdated
| @@ -333,7 +310,7 @@ def cast_long_to_signed_16(value): | |||
| Returns: | |||
| byte -- Int Signed value in range -128..127 | |||
| """ | |||
| assert(0 <= value <= 65535) | |||
| assert(0 <= value <= 65535), "Assert failed! Value doesn't fit 2 bytes" | |||
| output[ch][w][h] = data[i] | ||
| i = i + 1 | ||
| return output | ||
|
|
||
| def cast_long_to_signed_byte(value): |
There was a problem hiding this comment.
Not part of the review, but I wonder, how did this C-language ABI-specific type names (long) end up in our source tree 😅
Ideally, this would be something like cast_u64_to_i8. Besides, current function name leaves it up to interpretation whether it's a signed or an unsigned long. One has to wonder, why is the signed name-specifier included for the byte but not for the long.
There was a problem hiding this comment.
So the logic is that the input value is a Python long type, an integer of (almost) unlimited size. We want to sanitize the longs from Renode and reinterpret them to logical values in the range of -128..127. For naming, i8 would make sense here, but u64 is wrong. I don't know if this answers your question, but it's a necessary evil from working on a byte level in Python with Renode.
vp/devel/python_peripherals/DLA.py
Outdated
| @@ -1180,55 +1166,50 @@ def conv2d(self, input_img, kernels, padding, dilation=(1,1), stride=(1,1), padd | |||
|
|
|||
|
|
|||
| # Apply each kernel to input_img | |||
| first_pass = True | |||
vp/devel/python_peripherals/DLA.py
Outdated
| @@ -1274,7 +1255,7 @@ def matsum_element_wise(self, A, B): | |||
| if not isinstance(A, list) and not isinstance(B, list): | |||
| return A + B | |||
|
|
|||
| assert len(A) == len(B) | |||
| assert len(A) == len(B), "Assert failed! Different sized operands in matsum" | |||
There was a problem hiding this comment.
Assert style changed with respect to earlier code. Seemed to have parentheses before but not here.
There was a problem hiding this comment.
Style unified to have parentheses
vilukissa68
force-pushed
the
feat/dla-vp-overflow-fix
branch
from
2af43f2 to
a5b71d7
Compare
Fixes buffer overflow issue with bank switching. Adds some quality of life features to prints. Removes dead code.