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

Bitfield fixes #270

Merged
merged 3 commits into from Apr 14, 2014
Merged

Bitfield fixes #270

merged 3 commits into from Apr 14, 2014

Conversation

neobrain
Copy link
Member

The BitField class added in b4337a2 turned out to have some critical issues: Generated assembly code would be bloated and slow, and also the code had alignment issues when compiling on ARM. This branch contains two tiny patches which fix those issues by inlining all class methods and by ensuring proper structure packing. An additional third patch adds unit tests for the alignment fixes.

@neobrain
Copy link
Member Author

Space vs tabs fixes incoming...

@@ -99,6 +101,7 @@
* printf("Value: %d", (s32)some_register.some_signed_fields);
*
*/
#pragma pack(1)

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@delroth
Copy link
Member

delroth commented Apr 13, 2014

Other than this, LGTM.

@@ -158,3 +161,4 @@ struct BitField
static_assert(bits <= 8 * sizeof(T), "Invalid number of bits");
static_assert(bits > 0, "Invalid number of bits");
};

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

At least one platform (ARM with NEON instructions enabled) generates SIGBUSes if BitField objects aren't aligned properly.
@neobrain
Copy link
Member Author

No static_asserts, because alignof(storage) always returns 0, and while static_assert(__alignof(BitField<0,7,unsigned>) == 1, ""); does work I find it somewhat ugly, plus I'm not sure if checking for one specific template instantiation is even universal enough to cover the whole problem.

@neobrain
Copy link
Member Author

Added static_assert within the BitFieldTest.

delroth added a commit that referenced this pull request Apr 14, 2014
@delroth delroth merged commit fc71494 into dolphin-emu:master Apr 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants