[PATCH] D61654: [scudo][standalone] Introduce the chunk header

[Kostya Kortchinsky via Phabricator via llvm-commits]

cryptoad marked 5 inline comments as done.
cryptoad added inline comments.


================
Comment at: lib/scudo/standalone/chunk.h:42
+  u64 AllocType : 2;
+  u64 Offset : 16;
+};
----------------
vitalybuka wrote:
> why it uses u64 for 2bit fields?
I remembered having issues where the end structure was not ending up on a u64, but it seems to work with enums.
I changed the type as well to be a class enum.


================
Comment at: lib/scudo/standalone/chunk.h:58
+
+INLINE u16 computeChecksum(u32 Seed, uptr Value, uptr *Array, uptr ArraySize) {
+  // If the hardware CRC32 feature is defined here, it was enabled everywhere,
----------------
vitalybuka wrote:
> does this have to be inline?
Yes. This does matter when compiled with SSE4.2 for example, the resulting compiled code end up being 2 successive crc32 instructions without any extraneous calls.


================
Comment at: lib/scudo/standalone/chunk.h:69
+#else
+  if (atomic_load_relaxed(&HashAlgorithm) == HardwareCRC32) {
+    u32 Crc = computeHardwareCRC32(Seed, Value);
----------------
vitalybuka wrote:
> So you cant guaranty that it does not call computeChecksum before init() so you can avoid atomic operation?
I am not sure I understand this, do you mean I should not use atomics?
The function should never be called prior to initialization (malloc will call the allocator's init() before servicing an allocation).


Repository:
  rCRT Compiler Runtime

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61654/new/

https://reviews.llvm.org/D61654