[PATCH] D28417: [scudo] Separate hardware CRC32 routines

[Michał Górny via Phabricator via llvm-commits]

mgorny added inline comments.


================
Comment at: lib/scudo/scudo_allocator.cpp:85
   return Crc;
+
 }
----------------
Stray empty line, it seems.


================
Comment at: lib/scudo/scudo_crc32.cpp:42
+u32 computeCRC32(u32 Crc, uptr Data, u8 HashType) {
+  if (HashType == CRC32Hardware) {
+    return computeHardwareCRC32(Crc, Data);
----------------
This is still not entirely correct. Strictly speaking, the compiler can emit SSE4.2 instructions here, i.e. making them apply to pre-hardware or non-hardware call branch. However, it's highly unlikely it will use SSE4.2 to do comparison or function call, so I'm not going to insist on moving this elsewhere.

However, indirection into two function calls may actually affect performance, I think. Since hardware and software variants have the same signature, maybe it'd be better to replace the enum with function pointer, store hardware or software function in it and just call that function pointer in scudo_allocator?

Also, this reminds me I wanted to point out that ARM hardware variant is never used since hardware function is only called when SSE4.2 is available.


https://reviews.llvm.org/D28417