Skip to content

Conversation

@chedahub
Copy link
Contributor

@chedahub chedahub commented Oct 31, 2025

Description

This PR fixes a bug where the 16-bit signed immediate (si_split16) for VLE I16A-form instructions (e.g., e_add2i, e_add2is, e_mull2i, e_cmp16i, e_cmph16i, e_cmpl16i, e_cmphl16i) was decoded incorrectly.

The Bug

The upper 5 bits of the immediate (ui0_4) were being extracted from the wrong field in the instruction word. The code was using (word32 >> 16), which reads from the rD register field, instead of the correct (word32 >> 21).
image

The Fix

Modified vle32.c in the FillOperands32Vle function:
Changed the bit shift for ui0_4 from >>16 to >>21.
This ensures the si_split16 immediate value is now calculated correctly for all I16A-form instructions.
image

@chedahub chedahub changed the title [PPCVLE] Correct si_split16 immediate decoding for I16A-form inst… [PPCVLE] Correct si_split16 immediate decoding for I16A-form instructions Oct 31, 2025
@plafosse
Copy link
Member

plafosse commented Nov 4, 2025

Forgive me I'm not super familiar with this ISA but wouldn't this break I16L form instructions?

@chedahub
Copy link
Contributor Author

chedahub commented Nov 5, 2025

Hi @plafosse ,

Thank you for the review and for raising this point. That is a valid concern, as my change modifies a variable defined at the top of the FillOperands32Vle function.

You are correct that the I16L-form and I16A-form use different immediate encodings.

I've double-checked the code, and the I16L-form instructions (e.g., in case PPC_ID_VLE_E_LIS: and case PPC_ID_VLE_E_AND2I:) are unaffected by this change. As you can see in the code, these instructions locally re-define their own ui0_4 variable inside their respective case blocks, correctly using the (word32 >> 16) shift for their format.

My change to the function-scoped ui0_4 variable (and the si_split16 variable that depends on it) only fixes the I16A-form instructions (like e_add2i, e_cmp16i, etc.), which were the ones incorrectly using the top-level definition.

Since the I16L-form instructions handle their own immediate parsing locally, this change won't break their behavior.

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