-
Notifications
You must be signed in to change notification settings - Fork 9
feat: add padding utils functiun #1379
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
Conversation
byte_counts_test.accepts
Outdated
| {"rpad_128": { "input": [1329228075013078387168144653824294912], "res": [1329228075013078387168144653824294912] }} | ||
| {"rpad_128": { "input": [1329248278194519524574231007531630592], "res": [1329248278194519524574231007531630592] }} | ||
| {"rpad_128": { "input": [1334420292643450700532337556609564672], "res": [1334420292643450700532337556609564672] }} | ||
| {"rpad_128": { "input": [2658455991569831745807614120560689152], "res": [2658455991569831745807614120560689152] }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Intermediate test generation file accidentally committed to repository
The file byte_counts_test.accepts appears to be an intermediate artifact from the test data generation process. According to the comments in padding.go, this file is created as a temporary output (go run ... > byte_counts_test.accepts) before being shuffled into the actual test file at testdata/asm/util/padding.accepts. The test code references "asm/util/padding" which uses the file in testdata, not this root-level file. The mismatched name ("byte_counts" vs "padding") and its location outside the testdata directory further suggest this was unintentionally committed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree with cursor bot here. Looks strange the location of the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the file FYI
DavePearce
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. I remove the misc file, and also removed the label indents. And, should be merged shortly.
byte_counts_test.accepts
Outdated
| {"rpad_128": { "input": [1329228075013078387168144653824294912], "res": [1329228075013078387168144653824294912] }} | ||
| {"rpad_128": { "input": [1329248278194519524574231007531630592], "res": [1329248278194519524574231007531630592] }} | ||
| {"rpad_128": { "input": [1334420292643450700532337556609564672], "res": [1334420292643450700532337556609564672] }} | ||
| {"rpad_128": { "input": [2658455991569831745807614120560689152], "res": [2658455991569831745807614120560689152] }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree with cursor bot here. Looks strange the location of the file.
testdata/asm/util/padding.zkasm
Outdated
| if leading_zeroes != 0 goto exit_fail | ||
| res = rpad_120(remaining) | ||
| return | ||
| exit_fail: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
normally, labels in assembly languages are not indented so they stand out from other instructions. See e.g. here.
byte_counts_test.accepts
Outdated
| {"rpad_128": { "input": [1329228075013078387168144653824294912], "res": [1329228075013078387168144653824294912] }} | ||
| {"rpad_128": { "input": [1329248278194519524574231007531630592], "res": [1329248278194519524574231007531630592] }} | ||
| {"rpad_128": { "input": [1334420292643450700532337556609564672], "res": [1334420292643450700532337556609564672] }} | ||
| {"rpad_128": { "input": [2658455991569831745807614120560689152], "res": [2658455991569831745807614120560689152] }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the file FYI
Signed-off-by: F Bojarski <[email protected]>
Signed-off-by: F Bojarski <[email protected]>
Signed-off-by: F Bojarski <[email protected]>
This removes a misc testing filem, and also removes indents on labels to be consistent with assembly language style.
6337d1b to
fa58f32
Compare
Note
Adds right-padding zkasm utilities for u128/u120 with test integration and vectors; fixes minor comment typos in bit shift utility.
testdata/asm/util/padding.zkasmwithrpad_128,rpad_128_120, andrpad_120functions.Test_AsmUtil_Paddinginpkg/test/assembly_util_test.goto runasm/util/padding.testdata/asm/util/padding.goand vectorstestdata/asm/util/padding.accepts.testdata/asm/util/bit_shr.zkasm("decompose shift").Written by Cursor Bugbot for commit fa58f32. This will update automatically on new commits. Configure here.