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

bug(common): kmx struct alignment for more platforms 🙀 #9138

Closed
srl295 opened this issue Jun 30, 2023 · 4 comments
Closed

bug(common): kmx struct alignment for more platforms 🙀 #9138

srl295 opened this issue Jun 30, 2023 · 4 comments
Assignees
Milestone

Comments

@srl295
Copy link
Member

srl295 commented Jun 30, 2023

  • in kmx_file.h, this attribute is currently only applied on WASM builds (clang-based), but we need to consider for other compilers as well (clang-other platforms, GCC, VC++)

#9134 (comment)

@srl295 srl295 added this to the A17S17 milestone Jun 30, 2023
@srl295 srl295 self-assigned this Jun 30, 2023
@srl295
Copy link
Member Author

srl295 commented Jun 30, 2023

memdup'd in #9139

@srl295 srl295 closed this as not planned Won't fix, can't repro, duplicate, stale Jun 30, 2023
@srl295 srl295 changed the title fix(core): specify alignment for KMXPlus structs 🙀 bug(common): kmx struct alignment for more platforms 🙀 Jul 25, 2023
@srl295 srl295 modified the milestones: A17S17, A17S19 Jul 25, 2023
@srl295 srl295 reopened this Jul 25, 2023
@mcdurdin mcdurdin modified the milestones: A17S19, A17S22 Aug 18, 2023
@mcdurdin mcdurdin modified the milestones: A17S22, A17S23 Oct 1, 2023
@mcdurdin mcdurdin modified the milestones: A17S23, A17S24 Oct 15, 2023
@mcdurdin
Copy link
Member

Move

#ifdef __EMSCRIPTEN__
typedef KMX_DWORD __attribute__((aligned(1))) KMX_DWORD_unaligned;
typedef KMX_BOOL  __attribute__((aligned(1))) KMX_BOOL_unaligned;
typedef KMX_WORD  __attribute__((aligned(1))) KMX_WORD_unaligned;
#else
// TODO: consider other platforms
#define KMX_DWORD_unaligned KMX_DWORD
#define KMX_BOOL_unaligned  KMX_BOOL
#define KMX_WORD_unaligned  KMX_WORD
#endif

into km_types.h and then resolve the todo in it.

@mcdurdin
Copy link
Member

See also keyman_core_api_bits.h for cross-platform patterns we should copy for consistency.

@mcdurdin mcdurdin modified the milestones: A17S24, A17S25 Oct 27, 2023
srl295 added a commit that referenced this issue Nov 8, 2023
- move KMX struct alignment to km_types.h

For: #9138
srl295 added a commit that referenced this issue Nov 8, 2023
- add gcc / clang and msvc alignment options

For: #9138
@mcdurdin mcdurdin modified the milestones: A17S25, A17S26 Nov 13, 2023
srl295 added a commit that referenced this issue Nov 16, 2023
- move KMX struct alignment to km_types.h

For: #9138
srl295 added a commit that referenced this issue Nov 16, 2023
- add gcc / clang and msvc alignment options

For: #9138
@srl295
Copy link
Member Author

srl295 commented Nov 25, 2023

Fixed in #9977

@srl295 srl295 closed this as completed Nov 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants