Skip to content

[NFC][MC][XCore] Rearrange decoder functions for XCore disassembler #155009

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 26, 2025
Merged

[NFC][MC][XCore] Rearrange decoder functions for XCore disassembler #155009

merged 1 commit into from
Aug 26, 2025

Conversation

jurahul
Copy link
Contributor

@jurahul jurahul commented Aug 22, 2025 •
edited
Loading

Rearrange decode functions to be before including the generated disassembler code and eliminate forward declarations for most of them. This is possible because fieldFromInstruction is now in MCDecoder.h and not in the generated disassembler code.

@jurahul jurahul marked this pull request as ready for review August 22, 2025 19:02
@s-barannikov
Copy link
Contributor

Sorry, don't know what is going on, please see the snapshot of the comment.
image

@jurahul
Copy link
Contributor Author

jurahul commented Aug 22, 2025

Thanks, I lied :( The moved set of functions are also shuffled around a bit to avoid forward declarations within themselves (there were uses before definitions and I fixed them by rearranging the functions. No other code changes though. I updated description to mention that.

@nigelp-xmos
Copy link
Contributor

Happy to approve, but if this combines general tidying up of forward declarations, with changes enabled by fieldFromInstruction, wouldn't it be clearer in separate commits, or is there a reason to combine them? Minimizing NFCs?

@jurahul
Copy link
Contributor Author

jurahul commented Aug 26, 2025

Sounds good, I'll do this in 2 steps. (1) move all definitions up and remove any forward decls that are not necessary without reordering functions and then (2) reorder to eliminate some more fwd decls

@jurahul jurahul force-pushed the nfc_xcode_disasm_move_decoder_funcs branch from b6c7712 to b840e35 Compare August 26, 2025 16:46
@jurahul jurahul force-pushed the nfc_xcode_disasm_move_decoder_funcs branch from b840e35 to 22face3 Compare August 26, 2025 16:52
s-barannikov
s-barannikov approved these changes Aug 26, 2025
View reviewed changes
Copy link
Contributor

@s-barannikov s-barannikov left a comment

Choose a reason for hiding this comment

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

LGTM

@jurahul jurahul merged commit dac9d8f into llvm:main Aug 26, 2025
9 checks passed
@jurahul jurahul deleted the nfc_xcode_disasm_move_decoder_funcs branch August 26, 2025 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@s-barannikov s-barannikov s-barannikov approved these changes

@nigelp-xmos nigelp-xmos Awaiting requested review from nigelp-xmos

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jurahul @s-barannikov @nigelp-xmos