Skip to content
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

Consider converting assembly to use macros instead of raw .word, .halfword, etc. to avoid alignment bugs #2141

Open
MajikVroom opened this issue Nov 13, 2023 · 1 comment
Labels
Component: ASM/C Changes some internals of the ASM/C libraries Type: Maintenance Code style, infrastructure, updating dependencies

Comments

@MajikVroom
Copy link

Context: #2140

Data alignment bugs aren't obvious on review, and can be pretty subtle. Unfortunately armips doesn't seem to have any builtin way to specify "align by default" or anything like that, but it does support macros. If we broadly switched from raw .word 0x42 to a macro alignedword 0x42, defined as

.macro alignedword,value
.align 4
.word value

that could set a pattern where things are aligned as needed by default.

Disclaimer: I'm not deeply familiar with the asm/C side of the randomizer, and there might be some reason I'm not seeing why we'd regularly want to avoid forcing alignment. But it seems like this would be a decent way to get rid of a subtle pitfall (and it should make it possible to rip out some of the explicit .align directives currently scattered through the .asm)

Main downside I see is just having to deal with a bunch of merge conflicts in forks that touch the .asm files...

@fenhl fenhl added the Component: ASM/C Changes some internals of the ASM/C libraries label Nov 13, 2023
@cjohnson57
Copy link
Collaborator

Seems like a good idea to me.

@fenhl fenhl added the Type: Maintenance Code style, infrastructure, updating dependencies label Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: ASM/C Changes some internals of the ASM/C libraries Type: Maintenance Code style, infrastructure, updating dependencies
Projects
None yet
Development

No branches or pull requests

3 participants