Use bool for bitfield values

[ccawley2011]

The joypad_bits bitfield has been removed since it relied on specific positions, however the layout is not standardised and may differ between compilers: https://stackoverflow.com/questions/19376426/order-of-fields-when-using-a-bit-field-in-c

The rest of the bitfield have been changed to use unsigned int instead of uint8_t for compatibility with more compilers.

[deltabeard]

Which compilers require unsigned int for bitfields? Is it possible for those compilers to support unsigned char for bitfields instead? By using unsigned int, the bitfields will take 32-bit instead of 8-bit.

[ccawley2011]

Norcroft produces errors if unsigned char is used as a bit field type, and I get the impression that it's not part of the C99 standard: https://stackoverflow.com/questions/17670436/in-case-of-bit-fields-which-one-is-better-to-use-unsigned-char-or-unsigned-int

Regarding the total size of the bit fields, I did some investigation and it seems that if you use bitfield and non-bitfield values in the same structure, GCC at least will limit the size of the bitfields appropriately. As such, I think the f_bits structure (and the unnamed one) are the only ones that will change size with this PR - the rest of should either remain the same size or result in padding even without this PR. I'll investigate this in more detail tomorrow.

Compiler Explorer link

[deltabeard]

It seems that only the sizeof_direct_small() function returns a size of 1 for all three compilers tested (GCC ARM, GCC x86-64, MSVC x86). I would prefer not to use the pack pragma (MSVC doesn't even support it).

It seems that using bool for the bitfield produces the same results as using unsigned char and is also compatible to the C99 specification. See Compiler Explorer.

So the single bit bitfields should be changed to bool. Could you check if Norcroft compiler supports that?

[ccawley2011]

So the single bit bitfields should be changed to bool. Could you check if Norcroft compiler supports that?

It does, but it always pads the bitfield to 32 bits. This might cause a problem with using f_bits in a union, but should be OK for the rest of the bitfields.

[deltabeard]

The reg variable in the union with f_bits should also be changed to bool. That way, GCC and MSVC will optimise that union to a single byte. Norcroft not doing that shouldn't be an issue though, because the reg variable in the f struct is only used to zero all the bitfields in f_bits - there are no bitwise operations in Peanut-GB using reg. So I think setting reg as a bool and it being 32-bits on some systems shouldn't be an issue. Compiler Explorer

[deltabeard]

I didn't consider the 16-bit reg variables used for bc, de, etc. 🤦‍♂️ I'll need to put more thought into this. EDIT: Never mind, I was confusing myself. bc doesn't use bitfields so uint16_t should be just fine.

[ccawley2011]

So I think setting reg as a bool and it being 32-bits on some systems shouldn't be an issue.

bool is 8-bit with Norcroft when outside of a bitfield, so there's a potential issue where reg might not cover the area used by f_bits if it gets padded.

I've pushed an update that reverts the changes to f_bits for now and uses bool instead of uint8_t for the rest of the bitfields. I'll open a separate PR for dealing with f_bits.

[deltabeard]

I think that the changes to the joypad struct should also be isolated in its own pull request. In addition, the removal of the bitfields for the joypad would break the API compatibility, so it could only really be made a in a major version bump.

Is there a benefit to changing 0 to false?

I'll have to give more thought the to use of the joypad bitfields.

[ccawley2011]

I think that the changes to the joypad struct should also be isolated in its own pull request. In addition, the removal of the bitfields for the joypad would break the API compatibility, so it could only really be made a in a major version bump.

It's now in PR #106, which now deprecates the bitfield instead of removing it.

Is there a benefit to changing 0 to false?

Mostly just correctness, however the changes to the SDL2 example are needed to fix a warning.

[deltabeard]

Thanks!