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

Add definition of enum storage type #209

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

xypron
Copy link

@xypron xypron commented Aug 17, 2021

Some APIs use enum types in their definitions. The C specification does not
define the size of the integer type used for storing and passing enum type
variables.

Provide a definition matching GCC's default behavior.

Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com

riscv-elf.md Outdated
Comment on lines 450 to 452
`enum` types use int as storage type if the value range fits into the value
range of signed int or into the value range of unsigned int. If the value
range is larger, they use long long as storage type.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd put this just below char and bool above as they all relate to integral types, rather than interspersed between float-related things (float16 and complex).

Also, is long long correct, or should it be "smallest primitive integer type" so long gets used on RV32? Using unsigned int is a bit weird potentially too, would have to see what other ABIs do here.

Copy link
Author

Choose a reason for hiding this comment

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

@jrtc27:

I tested both on ARM 32bit and riscv64:

GCC uses 32 bit storage with 32bit alignment if the enum values are either

  • in [0, 2^32-1] or
  • in [-2^31, 2^31-1]
    Otherwise a 64 bit storage with 64bit alignment is chosen.

If the values do not fit into a 64bit representation this results in a "warning: enumeration values exceed range of largest integer".

I don't have an RV32 compiler but I would assume that GCC does the same as on ARM 32bit.

So long long seems to be correct.

Concerning the placement of the paragraph I have no preferences. Shall I resubmit with the same text? Or will you relocate when merging?

For testing on riscv32 you could use this program:

#include <stdio.h>
#include <limits.h>

enum e1 {
        A1 = 0,
        B1 = 0x7FFFFFFF,
};

enum e2 {
        A2 = 0,
        B2 = 0xFFFFFFFF,
};

enum e3 {
        A3 = -0x80000000,
        B3 = 0x7FFFFFFF,
};

enum e4 {
        A4 = 0,
        B4 = 0xFFFFFFFF,
};

enum e5 {
        A5 = 0,
        B5 = 0xFFFFFFFFFFFFFFFF,
};

enum e6 {
        A6 = -0x8000000000000000,
        B6 = 0x7FFFFFFFFFFFFFFF,
};

enum e7 {
        A7 = 0,
        B7 = 0xFFFFFFFFFFFFFFFF,
};

enum e8 {
        A8 = -0x8000000000000000,
        B8 = 0xFFFFFFFFFFFFFFFF,
};

int main()
{
        printf("e1: %zu, %zu\n", sizeof(enum e1), __alignof__(enum e1));
        printf("e2: %zu, %zu\n", sizeof(enum e2), __alignof__(enum e2));
        printf("e3: %zu, %zu\n", sizeof(enum e3), __alignof__(enum e3));
        printf("e4: %zu, %zu\n", sizeof(enum e4), __alignof__(enum e4));
        printf("e5: %zu, %zu\n", sizeof(enum e5), __alignof__(enum e5));
        printf("e6: %zu, %zu\n", sizeof(enum e6), __alignof__(enum e6));
        printf("e7: %zu, %zu\n", sizeof(enum e7), __alignof__(enum e7));
        printf("e8: %zu, %zu\n", sizeof(enum e8), __alignof__(enum e8));

        return 0;
}

Copy link
Collaborator

@jrtc27 jrtc27 Aug 17, 2021

Choose a reason for hiding this comment

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

Oops, I meant long on RV64, as that's sufficient for 64 bits. I realise it'll be a 64-bit type, but that doesn't tell you if it uses long or long long when the two have the same representation (which matters for printf format strings, C++ name mangling, overloaded functions, ...).

Using Clang is easiest, one compiler does everything. But godbolt.org has RV32+RV64 Clang+GCC.

Copy link
Author

Choose a reason for hiding this comment

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

On riscv64 %lx is required for printing. On armv7 only %llx delivers the correct output with GCC while %lx and %x are accepted but the output seems to be an address.

So the 64 bit types should be long on riscv64 and long long on riscv32.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, so it is indeed the lowest rank integer type whose range is large enough to represent it.

Copy link
Author

Choose a reason for hiding this comment

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

@jrtc27 I pushed a revised path some time ago. Could you, please, review.

Some APIs use enum types in their definitions. The C specification does not
define the size of the integer type used for storing and passing enum type
variables.

Provide a definition matching GCC's default behavior.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Comment on lines +446 to +451
`enum` types use `int` as storage type if the value range fits into the value
range of `signed int` or into the value range of `unsigned int`. If the value
range is larger, the enum values use an 64bit integer storage type:

* `long` for an LP64 calling convention,
* `long long` for an ILP32 calling convention.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs to actually specify the signedness of the types explicitly, not just that it uses an int's worth of space, as it affects things like C++ overloading. That is, it needs to be clearly:

  1. If it fits in an int, use int
  2. Else, if it fits in an unsigned int, use unsigned int
  3. Else, if it fits in a long (long), use long (long)
  4. Else use unsigned long (long)

(assuming that the long (long) case mirrors the int case in its choice of signedness)

@kito-cheng
Copy link
Collaborator

LGTM, @jrtc27 feel free to merge that once you are OK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants