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

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added inline comments.


================
Comment at: lib/scudo/standalone/chunk.h:30
+  ChunkAvailable = 0,
+  ChunkAllocated = 1,
+  ChunkQuarantine = 2
----------------
enum class ChunkState : u8 {
  Available = 0,
  Allocated = 1,
  Quarantine = 2,
};


================
Comment at: lib/scudo/standalone/chunk.h:39
+  u64 ClassId : 8;
+  u64 SizeOrUnusedBytes : 20;
+  u64 State : 2;
----------------
can you use your enums here instead of u64


================
Comment at: lib/scudo/standalone/chunk.h:42
+  u64 AllocType : 2;
+  u64 Offset : 16;
+};
----------------
why it uses u64 for 2bit fields?


================
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,
----------------
does this have to be inline?


================
Comment at: lib/scudo/standalone/chunk.h:69
+#else
+  if (atomic_load_relaxed(&HashAlgorithm) == HardwareCRC32) {
+    u32 Crc = computeHardwareCRC32(Seed, Value);
----------------
So you cant guaranty that it does not call computeChecksum before init() so you can avoid atomic operation?


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D61654