emscripten-core / emscripten

Emscripten: An LLVM-to-WebAssembly Compiler
Other
25.35k stars 3.25k forks source link

Replace EM_BOOL with normal C/C++ bool type. NFC #22155

Open sbc100 opened 3 days ago

sbc100 commented 3 days ago

There is no need that I can think of to use a custom type/macro here.

This also reduces the size of several structs due to the fact that the C/C++ bool type is smaller than int.

Keep EM_BOOL/EM_TRUE/EM_FALSE around for backwards compat but don't use them internally anymore.

sbc100 commented 3 days ago

This change was created using sed to substitutions

sbc100 commented 3 days ago

Updated to include struct size saves and code size savings.

sbc100 commented 3 days ago

I could do this in two phases perhaps:

kripken commented 3 days ago

Why do struct sizes change? I'd expect an int and an unpacked bool to both take 32 bits. Is that wrong?

sbc100 commented 3 days ago

Why do struct sizes change? I'd expect an int and an unpacked bool to both take 32 bits. Is that wrong?

I guess bool is just a single byte in C/C++?

Indeed this program printf 1 1 on my desktop system:

$ cat test.c 
#include <stdbool.h>
#include <stdio.h>

int main() {
  printf("%ld %ld\n", sizeof(bool), _Alignof(bool));
  return 0;
}
sbc100 commented 3 days ago

Why do struct sizes change? I'd expect an int and an unpacked bool to both take 32 bits. Is that wrong?

I guess bool is just a single byte in C/C++?

Indeed this program printf 1 1 on my desktop system:

$ cat test.c 
#include <stdbool.h>
#include <stdio.h>

int main() {
  printf("%ld %ld\n", sizeof(bool), _Alignof(bool));
  return 0;
}

(same for C++ BTW)

kripken commented 3 days ago

By itself it's 1 byte, I guess, but inside a struct the field after it might be aligned:

#include <stdbool.h>
#include <stdio.h>

class C {
  bool x;
  int y;
};

int main() {
  printf("%ld\n", sizeof(C));
  return 0;
}

That prints 8 for me, so 4 bytes for each field. I'd expect that to be the common case, I'm surprised we save anything actually...

kripken commented 3 days ago

Anyhow, yeah, if this causes struct layout changes then splitting the PR as much as possible sgtm.

sbc100 commented 3 days ago

By itself it's 1 byte, I guess, but inside a struct the field after it might be aligned:

#include <stdbool.h>
#include <stdio.h>

class C {
  bool x;
  int y;
};

int main() {
  printf("%ld\n", sizeof(C));
  return 0;
}

That prints 8 for me, so 4 bytes for each field. I'd expect that to be the common case, I'm surprised we save anything actually...

Right, that is just normal struct layout rules. If you put the bool last you will see difference. Or if you have several booleans toegther in the struct.

Lowering the alignment of a type can for sure shrink struct sizes. That is the primary use of the alignment of a given type. Higher alignment == worse for struct size.

sbc100 commented 1 day ago

Rebased now that #22157 has landed. I'm planning on waiting for a release or two before landing this larger change.