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

libutils: add kernel word size related CTZ macro #176

Closed
wants to merge 1 commit into from

Conversation

colorglass
Copy link

Add kernel word size related macros CxZ_WORD() for ctz and clz builtin func. And also add CxZLL() macro for 64bits data size.
And for the macro CTZL(), which is used for unsigned long type, I found a error use in sel4test timer dirver: https://github.com/seL4/sel4test/blob/0a488b92bc7a08acc14d49524d1f22656f286627/apps/sel4test-driver/src/timer.c#L53

......
void handle_timer_interrupts(driver_env_t env, seL4_Word badge)
{
    int error = 0;
    while (badge) {
        seL4_Word badge_bit = CTZL(badge);
......

The seL4_Word type can be 64bits, but CTZL() recives unsigned long argument. It can be changed to CTZ_WORD() to fix the problom.

@@ -29,6 +31,18 @@
#define BSWAP_WORD(x) __builtin_bswap64(x)
#endif

#if CONFIG_WORD_SIZE == 32
Copy link
Member

@axel-h axel-h Jan 31, 2024

Choose a reason for hiding this comment

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

Let's group the variants together and issue a warning if CONFIG_WORD_SIZE is invalid. Also do some sanity checking that the types match. Userland libraries could be compiles with any settings.

    #include "compile_time.h"

    ....

    #if CONFIG_WORD_SIZE == 32

    #define BSWAP_WORD(x) __builtin_bswap32(x)
    compile_time_assert(long_is_32_bit, sizeof(long) == 4)
    #define CTZ_WORD(x) CTZL(x)
    #define CLZ_WORD(x) CLZL(x)

    #elif CONFIG_WORD_SIZE == 64

    #define BSWAP_WORD(x) __builtin_bswap64(x)
    compile_time_assert(long_is_64_bit, sizeof(long long) == 8)
    #define CTZ_WORD(x) CTZLL(x)
    #define CLZ_WORD(x) CLZLL(x)

    #else
    /*
     *  This library aims to be agnostic of the "word" concept, so don't raise an
     *  error if CONFIG_WORD_SIZE is unsupported or not set. We just don't provide
     *  the macros at all in this case.
     */
    #endif

#if CONFIG_WORD_SIZE == 32
#define CTZ_WORD(x) CTZL(x)
#elif CONFIG_WORD_SIZE == 64
#define CTZ_WORD(x) CTZLL(x)
Copy link
Member

Choose a reason for hiding this comment

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

Actually, on 64-bit systems, it is quite common that long is 64-bit. So we could also go with #define CTZ_WORD(x) CTZL() in both cases. Note that In libutils/include/utils/arith.h this lib already uses CTZL().

Copy link
Author

Choose a reason for hiding this comment

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

You are right, but I tested it using both x64 and arm64 msvc 19.30 and get that long is 32bits size. So I suggest that we use the more likely selection for word size type.

Copy link
Member

Choose a reason for hiding this comment

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

Oh - if long is 32-bit on a 64-bit architecture, I'd expect this to break a lot more things...

Copy link
Member

Choose a reason for hiding this comment

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

long is definitely 64 on the compilers that are supported for the 64 bit architectures that are supported.

Add kernel word size related macros CxZ_WORD() for ctz and clz builtin
func. And also add CxZLL() macro for 64bits data size.

Signed-off-by: colorglass <55863235+colorglass@users.noreply.github.com>
@colorglass
Copy link
Author

colorglass commented Feb 1, 2024

Thanks for your attention.

I searched C99 standard ISO/IEC 9899:1999, and didn't find the size define of long, only got some information indicates that it should hold the number limits defined in limits.h.

And I find that the size of long in gcc was defined by a internal macro LONG_TYPE_SIZE Layout of Source Language Data Types, which defines that long type in gcc has the default one word size.

Finally I write the simple test file to print the result of sizeof(long), compile and run and get the result is 4 within my visual studio 2019.

So I can confirm that the practical size of the long is depended on the compiler itself. The most generally used compiler is gcc and one word size long becomes a de facto standard.

Please forgive my attention to details, and since there is no builtin_ctz() function in msvc, we can just apply CTZL() to word size variable with no change to this file.

@colorglass colorglass closed this Feb 1, 2024
@axel-h
Copy link
Member

axel-h commented Feb 1, 2024

See explanation here: https://en.m.wikipedia.org/wiki/64-bit_computing
Seems LLP64 support for userland is currently not planned then.

@axel-h axel-h reopened this Feb 1, 2024
@axel-h axel-h closed this Feb 1, 2024
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.

3 participants