-
Notifications
You must be signed in to change notification settings - Fork 5
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
Encoder identity refactoring changes #19
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit renamed all the enocder identitiy enums from the prev- ious `OP_` prefix to the `ENC_` prefix. Previously, the enocder identities were placed in the `operand.h` file and held the `OP_` prefix before it was moved to this file. However despite being moved to another module, the old name stayed around until it was finally renamed in this commit.
Previosuly, the enocder function definitions were literally copied and pasted into a header file and introduced lots of verbosity to the header, things like the encoder arguments were repeated many times and was hard to maintain, this commit rectifies this issue by creating a *temporary macro* that will create function defini- tions for you automatically using the standard argument library.
Previous name of the common function for the `MR` and `MR` encoder identities carried the un-descripitive and in conclusive name of `mr_rm_ref()`, this new name - `rm_mr_common` should better explain the funcitonality of this encoder helper function. Further more, this commit also removed the arbitary boolean param- eter value that described if the function was used to encode the `RM` or `MR` encoder identity, now this function simply takes a enumeration of the current identity and changes behavior based on this.
This commit migrated some functions and types from the `instruct- ion.h` header to the new `encoder.h` as well as removing the enocder function definition macros. Please see the detailed changelog below: - Since there is *no recursive* preprocessing in the normal C stand- ard, the previous macro, namely the `DECLARE_ENCODER_LIST` macro fails to work, therefore removed in favour of the migration of the `instr_encode_func` from the `instruction.h` header. - The `instr_encode_func` and `instr_encoder_t` function and type respectively has been moved over to the `encoder.h` file along with the renaming of their prefixes to `enc` (Which stands for encoder) rather than the old `instr` prefix (aka *instruction*) Now, the enocder functions can be accessed using the `enc_lookup` wrapper function so the function definition doesn't need to be declared in the `encoder.h` file and prevents namespace poullution issues as well as naming clashes.
As per the commit message in 64b99a5: > - The `instr_encode_func` and `instr_encoder_t` function and type > respectively has been moved over to the `encoder.h` file along with > the renaming of their prefixes to `enc` (Which stands for encoder) > rather than the old `instr` prefix (aka *instruction*) For somereason this was not carried over for the header definiton, causing linkage errors during linktime. This commit just matches this with the function prototype in the source file. (No more linker issues now :))
After the migration and renaming of the instruction encoder look- up function in the previous commit, the callers were not adjusted to the change. This commit adds the `encoder.h` header file instead (Since it moved) and changed name to match function call
IMO using the enocder type as a type for pre processors seems very confusing, so this commit defined the `encoder_t` type to another name sort of like a *alias* as the name of `pre_encoder_t`
cheng-alvin
added
enhancement
New feature or request
patch
Small and minor patch
refactor
Minor changes in favour for maintainability and/or readability
labels
Dec 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
enhancement
New feature or request
patch
Small and minor patch
refactor
Minor changes in favour for maintainability and/or readability
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This Pull, the encoder module received minor updates and changes including function and enum renaming as well as minor refactoring works carried out. Notably, the lookup table for encoders has been moved from the instruction module over to the encoder module and all encoder functions must be call through this function, encoder functions shall not be called directly. More details appear in the commit logs