llama2.c icon indicating copy to clipboard operation
llama2.c copied to clipboard

Use of standard C integers defined in stdint.h

Open rdentato opened this issue 2 years ago • 3 comments

Rather than relying on compilers to choose the right size for integers, let's call them out explicitly using the types defined in the standard header stdint.h.

I believe this makes things clearer and avoid some subtle bug on special architectures.

rdentato avatar Aug 21 '23 13:08 rdentato

I am really not sure about this branch. For me all of the _t stuff is visual clutter, it makes it harder for me to parse the code. Maybe we can assert fail if int < 4 bytes? Correct me if I'm wrong but basically all major compilers today will have size of int = 4. Is this an actual issue? E.g. since the start of this repo I believe noone has complained about this.

karpathy avatar Aug 21 '23 15:08 karpathy

I believe, for example, that int64_t is clearer than long. However consider that I come from an age when usually int was 16 bits and I still have to switch between sizes. In any case, some assert() at the beginning to ensure that long = 64 bits, long long = 64 bits, and int = 32 bits would be fine. I see lama2.c compiled on many platforms and using stdint.h is the standard way to ensure you get what you expect to get.

rdentato avatar Aug 21 '23 15:08 rdentato

I personally like intN_t stuff when there is a requirement on the number of bits, and plain int when there are no such requirements, which also shows explicitly in the code where such requirements are present and what they are.

As a practical matter, right now int is always 32 bit on every platform we care about, but long is 32 bit on MSVC and gcc -m32 will also make long to be 32 bit.

And INT8 quantized weights should really be int8_t.

ozabluda avatar Aug 21 '23 18:08 ozabluda