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

kernel: structs: define priq bitmap size via bits per long #72626

Conversation

cfriedt
Copy link
Member

@cfriedt cfriedt commented May 11, 2024

Since a long is 64-bits on 64-bit architectures, and 32-bits on 32-bit architectures, we can simplify the definition of PRIQ_BITMAP_SIZE by defining it in terms of BITS_PER_LONG.

andyross
andyross previously approved these changes May 13, 2024
@npitre
Copy link
Collaborator

npitre commented May 13, 2024 via email

@cfriedt
Copy link
Member Author

cfriedt commented May 15, 2024

I think this is not enough. If you are changing this to a long, you
should also change the actual bitmap type to a long in struct _priq_mq
to match.

Correct - let me fix that.

Since a long is 64-bits on 64-bit architectures, and 32-bits
on 32-bit architectures, we can simplify the definition of
PRIQ_BITMAP_SIZE by defining it in terms of BITS_PER_LONG.

Signed-off-by: Chris Friedt <cfriedt@tenstorrent.com>
@cfriedt cfriedt force-pushed the define-priq-bitmap-size-by-bits-per-long branch from 242a649 to 28b9081 Compare May 16, 2024 00:06
@cfriedt cfriedt requested review from TaiJuWu and andyross May 16, 2024 00:06
@cfriedt
Copy link
Member Author

cfriedt commented May 16, 2024

I think this is not enough. If you are changing this to a long, you should also change the actual bitmap type to a long in struct _priq_mq to match.

@npitre - Fixed!

Copy link
Contributor

@andyross andyross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems clean. Though pedants will point out that "long" is in fact 32 bits on x86_64 Windows toolchains for compatibility, leading to suboptimal code if someone ever gets the urge to try to build Zephyr with MSVC.

@cfriedt
Copy link
Member Author

cfriedt commented May 16, 2024

Seems clean. Though pedants will point out that "long" is in fact 32 bits on x86_64 Windows toolchains for compatibility, leading to suboptimal code if someone ever gets the urge to try to build Zephyr with MSVC.

That would be something 😅

I had considered using uintptr_t - it's not too late 🤷🏼‍♂️

@carlescufi carlescufi merged commit ed501d7 into zephyrproject-rtos:main May 16, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants