Skip to content

test: add valid positive and negative J-type immediate encoding tests to encoder#606

Open
amathxbt wants to merge 1 commit into
nexus-xyz:mainfrom
amathxbt:fix/encoder-j-type-test-coverage
Open

test: add valid positive and negative J-type immediate encoding tests to encoder#606
amathxbt wants to merge 1 commit into
nexus-xyz:mainfrom
amathxbt:fix/encoder-j-type-test-coverage

Conversation

@amathxbt
Copy link
Copy Markdown

@amathxbt amathxbt commented May 7, 2026

Problem

test_j_type_jump_boundaries uses op_c = 1048576 (2^20) for the “positive” case and op_c = -1048576i32 as u32 for the “negative” case, then asserts both produce 0x800000EF.

This is not a sign error — it is a two’s-complement identity: 2^20 equals the minimum representable value in a 21-bit signed immediate field (the J-type field width), so the two op_c values are bit-for-bit identical. The test never exercises any valid negative jump offset, leaving the encoder’s negative-immediate path entirely untested.

The B-type test (test_b_type_branch_offsets) correctly tests a negative offset (BEQ x1, x2, -16); the J-type test should do the same.

Fix

Add two concrete test cases that cover valid positive and negative J-type immediates:

Instruction op_c Expected encoding
JAL x1, +16 16 0x10000EF
JAL x1, -8 -8i32 as u32 0xFF9FF0EF

Expected values are derived from the RISC-V J-type bit-field layout (spec §2.5) and verified manually. The original ±1 MB case is retained with a comment explaining the two’s-complement identity.

test_j_type_jump_boundaries previously used 1048576 (2^20) as both "positive"
and "negative" 1 MB jumps. Because 2^20 equals the minimum value in a 21-bit
two's-complement immediate field, both op_c values produce the same 32-bit
encoding (0x800000EF), so the test did not exercise the encoder's handling of
any valid negative offset.

Replace the ambiguous test cases with:
- JAL x1, +16  (valid positive) -> 0x10000EF
- JAL x1, -8   (valid negative) -> 0xFF9FF0EF

The expected values are derived from the RISC-V J-type bit-field layout and
confirmed manually. The original ±1 MB case is retained with a comment
explaining the two's-complement identity.
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 7, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@amathxbt
Copy link
Copy Markdown
Author

amathxbt commented May 7, 2026

I have read the CLA Document and I hereby sign the CLA

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.

1 participant