Skip to content

Conversation

amkCha
Copy link
Collaborator

@amkCha amkCha commented Oct 13, 2025

Handles the case of instruction SIGNEXTEND in BIN module

Adds 1000 traces for INST=11 extracted from bin.all.bz2 in bin.accepts

Note : at first, I thought I needed the index of the word's first non null byte, which I don't. Hence the util function first_non_null_byte - I kept it as it could be useful, but if you'd rather keep the utils lightweight, let me know 👌


Note

Adds SIGNEXTEND handling to bin and introduces signextend and first_nonzero_byte utilities with extensive test vectors.

  • ASM BIN module:
    • Handle SIGNEXTEND (INST=11) in testdata/asm/bench/bin.zkasm: add dispatch to signextend_call, return unchanged for oversized index, and include ../util/signextend.zkasm.
  • Utilities:
    • Add testdata/asm/util/signextend.zkasm implementing signextend(size, word) using byte256 and fill_bytes_between.
    • Add testdata/asm/util/first_byte.zkasm implementing first_nonzero_byte helpers for u256 down to u16.
  • Test vectors/bench data:
    • Append large set of INST=11 cases to testdata/asm/bench/bin.accepts.
    • Add accepts for signextend in testdata/asm/util/signextend.accepts and for first_nonzero_byte in testdata/asm/util/first_byte.accepts.
  • Go tests:
    • Add Test_AsmUtil_FirstByte and Test_AsmUtil_Signextend in pkg/test/assembly_util_test.go.

Written by Cursor Bugbot for commit 55c808a. This will update automatically on new commits. Configure here.

@amkCha amkCha marked this pull request as ready for review October 13, 2025 20:08
Copy link
Collaborator

@DavePearce DavePearce left a comment

Choose a reason for hiding this comment

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

Yup, this looks good. I didn't find any issues. The only tiny thing I could find would be that we could increase the number of specific tests for util/signextend.zkasm. But, I don't think its necessary as we already have a lot coming from the tests for the bin module.

@amkCha
Copy link
Collaborator Author

amkCha commented Oct 14, 2025

Yup, this looks good. I didn't find any issues. The only tiny thing I could find would be that we could increase the number of specific tests for util/signextend.zkasm. But, I don't think its necessary as we already have a lot coming from the tests for the bin module.

Indeed, it was a little light, so I ended up added 250 randomly picked traces extracted from the same source - they should be slightly different than the traces from the bin module

@amkCha amkCha enabled auto-merge (squash) October 14, 2025 19:23
@amkCha amkCha merged commit 498cae7 into main Oct 14, 2025
14 checks passed
@amkCha amkCha deleted the feat/signextend-in-asm branch October 14, 2025 19:47
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.

2 participants