wwylele / teakra

DSi/3DS DSP emulator, disassembler, assembler, and tester
MIT License
76 stars 19 forks source link

Unbreak build on x86 #35

Closed jbeich closed 5 years ago

jbeich commented 5 years ago

Fixes #34

wwylele commented 5 years ago

Do you know where the layout difference come from? If it is a missing/extra padding in between some members, adding padding field at the end is incorrect; if it is just size difference due to not respecting u64 8-byte alignment, alignas(8) might be a better way to go.

wwylele commented 5 years ago

Closing due to inactivity

jbeich commented 4 years ago

I confirm, adding alignas(8) helps. Can you reopen or cherry-pick 5862fd000353?

wwylele commented 4 years ago

Github doesn't allow me to reopen this. Please open a new PR and let CI check stuff because I currently don't have the dev environment set up.

wwylele commented 4 years ago

Also adding alignas(8) on the top level struct isn't always correct either. It ensures the correct total size, but doesn't ensure the inner layout (for example all fields after a u64 field might be offset by 4 bytes). It should be added to all u64-type fields. A simple example:

struct { u32 a; u64 b; u32 c; u32 d;}

This struct on x64 would be layout as

| a: 4 bytes | padding: 4 bytes | b: 8 bytes | c: 4 bytes | d: 4 bytes |

which is 24 bytes in total.

On x86 where u64 alignment is 4-byte, it would be layout as

| a: 4 bytes | b: 8 bytes | c: 4 bytes | d: 4 bytes |

which is 20 bytes in total.

Adding alignas(8) on the top level (alignas(8) struct { u32 a; u64 b; u32 c; u32 d;}):

| a: 4 bytes | b: 8 bytes | c: 4 bytes | d: 4 bytes | padding: 4 bytes |

which is 24 bytes in total, but has a different layout from the x64 one (notice the different position of the padding)

Adding alignas(8) on the field (struct { u32 a; alignas(8) u64 b; u32 c; u32 d;}):

| a: 4 bytes | padding: 4 bytes | b: 8 bytes | c: 4 bytes | d: 4 bytes |

This produce the same layout as the x64 one