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

Registers should be defined as unions of bit-struct and u16 #529

Open
LIJI32 opened this issue Jan 13, 2018 · 5 comments
Open

Registers should be defined as unions of bit-struct and u16 #529

LIJI32 opened this issue Jan 13, 2018 · 5 comments

Comments

@LIJI32
Copy link
Contributor

LIJI32 commented Jan 13, 2018

I'm working on decompiling rock.c and I've noticed that the C code for do_boulder_dust used REG_BG1CNT as a bit-struct instead of an integer.

Specifically, this C statement REG_BG1CNT.priority = 1; (used by do_boulder_dust) translates to:

LDR             R4, =0x400000A
LDRB            R1, [R4]
MOVS            R0, #4
NEGS            R0, R0
ANDS            R0, R1
MOVS            R1, #1
ORRS            R0, R1
STRB            R0, [R4]

While the equivalent yet more arithmetic version REG_BG1CNT = (REG_BG1CNT & ~3) | 1; (used by sub_809EC38 from pokemon_summary_screen.c) translates to:

LDR             R4, =0x400000A
LDRH            R1, [R4]
LDR             R0, =0xFFFC
ANDS            R0, R1
MOVS            R1, #1
ORRS            R0, R1
STRH            R0, [R4]
LDRH            R1, [R4]

To properly decompile that function, the REG_BGXCNT registers must be redefined as unions, so they can be used as both integers and bit structs. Meanwhile, in order to avoid refactoring pretty much everything else in order to decompile this function, I'm using ((struct BGCNT *) &REG_BG1CNT) as a temporary hack.

@camthesaxman
Copy link
Contributor

Some of the code does use a bitfield, but other parts assign the values directly, and using a bitfield won't match. We already have a vBgCnt struct in types.h for this.

@LIJI32
Copy link
Contributor Author

LIJI32 commented Jan 15, 2018

Exactly, which is why they should be defined as unions. It's good to know these are already defined in types.h, so I don't need to redefine them. Additionally, do_boulder_dust also uses the DMA3 registers as structs of 3 u16s:

    REG_DMA3.SAD = (u32) &var_c;
    REG_DMA3.DAD = r1;
    REG_DMA3.CNT = 0x85000400;

@RevoSucks
Copy link
Collaborator

Isnt that just a DmaFill32 ? Please use the macros defined in gba/macros.h

@LIJI32
Copy link
Contributor Author

LIJI32 commented Jan 15, 2018

Turns out it is, thanks!

@camthesaxman
Copy link
Contributor

Oh, I see what you mean about making it a union. I believe that would match at the cost of being slightly more verbose because of the extra member required. If there's a significant amount of code that accesses it as a bitfield, then the change is probably worth it. If the vast majority of code accesses it as a raw u16, then it might be good to leave it as it is.

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

No branches or pull requests

3 participants