don't click here

Writing portable C

Discussion in 'Technical Discussion' started by BenoitRen, Mar 1, 2024.

  1. BenoitRen

    BenoitRen

    Tech Member
    795
    392
    63
    I've always been interested in doing things 'right', so Clownacy's articles on C were of great interest to me. In preparation for beginning in earnest on a C port of Sonic & Knuckles Collection, I decided to make this thread.

    Things I know to keep in mind:
    • char is one byte, and at least 8 bits
    • short is at least 16 bits
    • int is at least 16 bits
    • long is at least 32 bits
    • don't use C99 fixed-size types
    • don't use unions
    • don't assume endianness
    What worries me at the moment is the free part (actfree) of Sonic games' sprite status table. The way it is used makes assumptions on the size of the data. I've also just read that a pointer isn't always the same size as an integer, and may even differ in size depending on what it points to. Yikes!

    Anything else I should keep in mind?
     
  2. Clownacy

    Clownacy

    Tech Member
    1,099
    689
    93
    • 'char' is signed or unsigned depending on the target CPU. For instance, it is signed on x86 and unsigned on ARM. Do not assume that 'char' is signed.
    • '#pragma once' is not standard C; use manual header guards instead.
    • Bit-shifting signed integers is not well-defined, so multiply and divide them by a power of two instead, if you must.
    In my Sonic 2 C port, I made the free part of the sprite status table into a union of arrays of various types, like this:
    Code (Text):
    1.  
    2.     union
    3.     {
    4.         // 0x16 bytes of scratch RAM, to be done with as the object pleases
    5.         struct
    6.         {
    7.             uint8_t objoff_2A;
    8.             uint8_t objoff_2B;
    9.             uint8_t objoff_2C;
    10.             uint8_t objoff_2D;
    11.             uint8_t objoff_2E;
    12.             uint8_t objoff_2F;
    13.             uint8_t objoff_30;
    14.             uint8_t objoff_31;
    15.             uint8_t objoff_32;
    16.             uint8_t objoff_33;
    17.             uint8_t objoff_34;
    18.             uint8_t objoff_35;
    19.             uint8_t objoff_36;
    20.             uint8_t objoff_37;
    21.             uint8_t objoff_38;
    22.             uint8_t objoff_39;
    23.             uint8_t objoff_3A;
    24.             uint8_t objoff_3B;
    25.             uint8_t objoff_3C;
    26.             uint8_t objoff_3D;
    27.             uint8_t objoff_3E;
    28.             uint8_t objoff_3F;
    29.         } scratch8u;
    30.         struct
    31.         {
    32.             int8_t objoff_2A;
    33.             int8_t objoff_2B;
    34.             int8_t objoff_2C;
    35.             int8_t objoff_2D;
    36.             int8_t objoff_2E;
    37.             int8_t objoff_2F;
    38.             int8_t objoff_30;
    39.             int8_t objoff_31;
    40.             int8_t objoff_32;
    41.             int8_t objoff_33;
    42.             int8_t objoff_34;
    43.             int8_t objoff_35;
    44.             int8_t objoff_36;
    45.             int8_t objoff_37;
    46.             int8_t objoff_38;
    47.             int8_t objoff_39;
    48.             int8_t objoff_3A;
    49.             int8_t objoff_3B;
    50.             int8_t objoff_3C;
    51.             int8_t objoff_3D;
    52.             int8_t objoff_3E;
    53.             int8_t objoff_3F;
    54.         } scratch8s;
    55.         struct
    56.         {
    57.             uint16_t objoff_2A;
    58.             uint16_t objoff_2C;
    59.             uint16_t objoff_2E;
    60.             uint16_t objoff_30;
    61.             uint16_t objoff_32;
    62.             uint16_t objoff_34;
    63.             uint16_t objoff_36;
    64.             uint16_t objoff_38;
    65.             uint16_t objoff_3A;
    66.             uint16_t objoff_3C;
    67.             uint16_t objoff_3E;
    68.         } scratch16u;
    69.         struct
    70.         {
    71.             int16_t objoff_2A;
    72.             int16_t objoff_2C;
    73.             int16_t objoff_2E;
    74.             int16_t objoff_30;
    75.             int16_t objoff_32;
    76.             int16_t objoff_34;
    77.             int16_t objoff_36;
    78.             int16_t objoff_38;
    79.             int16_t objoff_3A;
    80.             int16_t objoff_3C;
    81.             int16_t objoff_3E;
    82.         } scratch16s;
    83.         struct
    84.         {
    85.             union
    86.             {
    87.                 struct
    88.                 {
    89.                     uint16_t filler1;
    90.                     uint32_t objoff_2C;
    91.                     uint32_t objoff_30;
    92.                     uint32_t objoff_34;
    93.                     uint32_t objoff_38;
    94.                     uint32_t objoff_3C;
    95.                 };
    96.                 struct
    97.                 {
    98.                     uint32_t objoff_2A;
    99.                     uint32_t objoff_2E;
    100.                     uint32_t objoff_32;
    101.                     uint32_t objoff_36;
    102.                     uint32_t objoff_3A;
    103.                     uint16_t filler2;
    104.                 };
    105.             };
    106.         } scratch32u;
    107.         struct
    108.         {
    109.             union
    110.             {
    111.                 struct
    112.                 {
    113.                     int16_t filler1;
    114.                     int32_t objoff_2C;
    115.                     int32_t objoff_30;
    116.                     int32_t objoff_34;
    117.                     int32_t objoff_38;
    118.                     int32_t objoff_3C;
    119.                 };
    120.                 struct
    121.                 {
    122.                     int32_t objoff_2A;
    123.                     int32_t objoff_2E;
    124.                     int32_t objoff_32;
    125.                     int32_t objoff_36;
    126.                     int32_t objoff_3A;
    127.                     int16_t filler2;
    128.                 };
    129.             };
    130.         } scratch32s;
    131.     };
    132.  
    In hindsight, I think this was a terrible idea: not only does it lead to difficult-to-read code, but it uses C99's fixed-width integer types.

    Instead, I think a better solution would be to make this so-called "scratch RAM" an array of unsigned chars, and use getters and setters to pack integers into these bytes. That way, regardless of the target CPU, a longword will always use 4 bytes, just like on a Mega Drive.
    Code (Text):
    1.  
    2. #define timedelay 0x30
    3. #define already_fired 0x31
    4. #define near_sonic 0x32
    5.  
    6. ObjectMove(sst);
    7. if (GetScratchRAM_U8(already_fired) || GetScratchRAM_U8(near_sonic))
    8.     return;
    9.  
    10. int player_distance = MainCharacter.x_pos - sst->x_pos;
    11. if (player_distance < 0)
    12.     player_distance = -player_distance;
    13.  
    14. if (player_distance >= 0x60 || !sst->render_flags.on_screen)
    15.     return;
    16.  
    17. SetScratchRAM_S8(near_sonic, true);
    18. SetScratchRAM_S8(already_fired, false);
    19. SetScratchRAM_S8(timedelay, 29);
    20.  
    Alternatively, if you prioritise performance and code simplicity over having the RAM perfectly match a Mega Drive, you could make the scratch RAM a union between various structs, one for each object, like this:

    Code (Text):
    1.  
    2. union
    3. {
    4.     struct
    5.     {
    6.         signed char timedelay;
    7.         unsigned char already_fired;
    8.         unsigned char near_sonic;
    9.     } buzz_bomber;
    10.     struct
    11.     {
    12.          unsigned char timesr[2];
    13.     } whisp;
    14. } scratch;
    15.  
    I don't know of any portability problems with using unions, so long as you're not using them to store data as one type and retrieve it as another.
     
    • Useful Useful x 2
    • Like Like x 1
    • Agree Agree x 1
    • List
  3. BenoitRen

    BenoitRen

    Tech Member
    795
    392
    63
    Weird that the standard leaves the default signage up for interpretation when it comes to char, but not the other numeric types. An int is always a signed int.

    What this tells me is that one should avoid "char", and use "signed char" and "unsigned char", instead.
    From my experience with decompiling Sonic CD, they're used to read and write individual bytes of larger values, which I think counts as storing data as one type and retrieving it as another. At the very least, this way of working leads to endianness problems.

    I was thinking that an alternative would be to use bit-shifting and masks to get at an individual byte instead, but if bit-shifting signed values is problematic, that won't always be an option.

    Side note: just read that C++11 goes one step further and says that left-bit-shifting signed integers is undefined. Right-bit-shifting signed integers is implementation-defined.
     
  4. synchronizer

    synchronizer

    Member
    2,306
    112
    43
    What is wrong with the standard fixed-width types (e.g. int32_t)? Don't those achieve what you want? (guaranteed size for a given integral type)
     
  5. BenoitRen

    BenoitRen

    Tech Member
    795
    392
    63
    After doing research I have to say it makes sense that bit-shifting negative numbers is not defined. We're used to a sign bit and two's complement, but there are more ways to implement negative numbers, and C gives that freedom.

    I found the following code to convert an unsigned number to a signed number in a portable way:
    Code (Text):
    1. if ( ret <= INT_MAX )
    2.    ret_as_signed = ret;
    3. else
    4.    ret_as_signed = -(int)(UINT_MAX - ret) - 1;
    The other way around is basically taking the absolute value:
    Code (Text):
    1. if (ret < 0) return ret * -1;
    2. else return ret;
    Read this: Stop Using Fixed-Width Integer Types!
     
  6. synchronizer

    synchronizer

    Member
    2,306
    112
    43
    It 404s, or actually all browsers complain it's an insecure site. Maybe you could just paste what I'm meant to read.
     
  7. Brainulator

    Brainulator

    Regular garden-variety member Member
  8. BenoitRen

    BenoitRen

    Tech Member
    795
    392
    63
    No problems here. Maybe you could try harder? I'm not going to paste an entire article.
     
  9. synchronizer

    synchronizer

    Member
    2,306
    112
    43
    I meant you could've just shared a sentence about it.
    Anyway, clownacy's site is blocked on secure/academic WIFI apparently. I have to go "off the grid."

    EDIT: okay then use the fast variants of the standard types. The issue there is that sometimes you need to guarantee exact same data layout when working between CPU code and GPU code. Then you have to assert that the sizes are the same, or you have to know your hardware better.
     
    Last edited: Mar 4, 2024
  10. BenoitRen

    BenoitRen

    Tech Member
    795
    392
    63
    I don't know, but it doesn't matter. I was just explaining the reasoning behind bit-shifting signed integers being implementation-defined. Which means we can't rely on compilers implementing it in the same way, even if they do use two's complement for signed numbers.

    By the way, I've since read that C++20 already mandates that signed numbers use two's complement.
     
  11. MainMemory

    MainMemory

    Kate the Wolf Tech Member
    4,795
    383
    63
    SonLVL
    In order to make Sonic & Knuckles Collection's code adhere to all these constraints, you're basically going to have to rewrite it entirely.
     
  12. BenoitRen

    BenoitRen

    Tech Member
    795
    392
    63
  13. MainMemory

    MainMemory

    Kate the Wolf Tech Member
    4,795
    383
    63
    SonLVL
    Maybe not everything, but there are certainly a lot of pointer arithmetic based on exact data sizes, storing data as one size and reading it as another, signed/unsigned arithmetic shenanigans, etc. Quite a few things will need adjustment if the 68000 RAM block is not exactly 0x10000 bytes and aligned on a boundary of such. How do you read something like S3K's level layout header in a platform-agnostic way?
     
  14. BenoitRen

    BenoitRen

    Tech Member
    795
    392
    63
    If the memory needs to be the same as the Mega Drive's, I've already got a problem. Just like in Sonic Jam, the sprite status tables take 2 more bytes each as they need to align on a 4 byte boundary.
     
  15. BenoitRen

    BenoitRen

    Tech Member
    795
    392
    63
    The reason that the sprite status table has to align on a 4 byte boundary is because the first data member is a memory pointer of 4 bytes. I researched if this was also a requirement on the 68000 CPU, but it looks like the only requirement is that the value is aligned on a word (2 byte) boundary.

    x86 does support unaligned memory access, which is probably why the machine-translated ASM works fine. PowerPC does as well, but for ARM it varies.

    So, if I really wanted to keep the memory layout the same, I'd have to replace the pointer by an array of four chars, which I'd cast to a pointer when reading or writing.
     
  16. MainMemory

    MainMemory

    Kate the Wolf Tech Member
    4,795
    383
    63
    SonLVL
    That'd fail anyway as soon as you try porting it to a platform that doesn't use 32-bit pointers. Like I said, there are going to have to be a lot of rewrites if you want to make it truly portable, because there are almost no guarantees when it comes to different CPUs. Technically, even 8 bits being a basic unit of memory isn't a universal truth, it's just something all the current CPU manufacturers agreed on.
     
    • Agree Agree x 2
    • Like Like x 1
    • List
  17. Glitch

    Glitch

    Tech Member
    176
    13
    18
    If you're targeting modern day, general purpose, consumer hardware then there's no reason not to use fixed width integer types, if that's what your data type calls for. The article seems to imply that there's some kind of negative performance impact involved in trying to use 16-bit arithmetic on a >=32-bit platform. This is simply not the case for general purpose desktop or mobile hardware. If you find that you're constantly having to write code to manually mask an unsigned value to 16-bits, then that's a good indication that you should probably be using a uint16_t. The compiler will add the masking for you, reducing the chance of you making mistakes, and ensure the correct overflow behaviour (signed overflow is UB - don't do that).

    Both x64 and Arm have instructions for masking and sign-/zero-extension that are single-cycle and can be parallel issued (ignoring data depenedency hazards). Memory alignment, cache locality, branch prediction and instruction scheduling are going to be performance concerns long before bitmasking a register is even a blip on the profiler's radar.

    I specifically mentioned "general purpose", multiple times, above because outside of those platforms YMMV. If you're writing code for a DSP, for example, then you're going to have to work with whatever the hardware gives you, and you may or may not have fixed-width types in your stdlib. The portability argument stands in this case, but do you really need your general purpose desktop program to also be portable to a DSP? Probably not. Be pragmatic, not dogmatic.
     
  18. BenoitRen

    BenoitRen

    Tech Member
    795
    392
    63
    There's at least one good reason: the fixed width integer types aren't guaranteed to exist.
     
  19. Glitch

    Glitch

    Tech Member
    176
    13
    18
    If you're compiling for Windows, macOS, Linux, BSD, Android, Haiku, etc. with clang, gcc or msvc then there's a very, *very* good chance that they do exist. That's why I mentioned modern, general purpose platforms and it's also why I added that last paragraph. If you need to target a platform where they're not availble then, obviously, you can't use them. If you're targeting platforms where they _are_ available and their usage reduces the chance of bugs, and communicates the intention of code more clearly, then why not use them?
     
    • Like Like x 3
    • Agree Agree x 3
    • List
  20. BenoitRen

    BenoitRen

    Tech Member
    795
    392
    63
    The thread is about writing portable C. If I targeted a subset of platforms, it wouldn't be portable.