[PATCH] D121510: [Support] Introduce the BLAKE3 hashing function implementation

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added inline comments.


================
Comment at: llvm/include/llvm/Support/BLAKE3.h:16
+
+#include "llvm-c/blake3.h"
+#include "llvm/ADT/ArrayRef.h"
----------------
akyrtzi wrote:
> dexonsmith wrote:
> > Might be nice to bury this `#include` in the source file to avoid polluting the global namespace with the C stuff. Inlining would still be available for release builds (assuming they use ThinLTO).
> Including the header is useful to avoid needing to allocate the C structures on the heap.
Good point.

Could separate it out (put a big enough aligned char buffer in the header), but maybe not worth it.

Also might be worth considering making the stack allocation optional. It's clocking in at 1912B, which is pretty big. Some users might want it on the heap.

That'd result in something like:
```
lang=c++
// Heap by default.
class BLAKE3 {
public:
  BLAKE3() = default;

protected:
  struct StorageT { alignas(alignof(uint64_t)) char Impl[1912]; };
  BLAKE3(StorageT &Storage); // for SmallBLAKE3.

private:
  // Allocate and init Hasher/OwnedHasher on first use.
  llvm_blake3_hasher &getOrCreate();

  llvm_blake3_hasher *Hasher = nullptr;
  std::unique_ptr<StorageT> OwnedHasher;
};

// Stack.
class SmallBLAKE3 : public BLAKE3 {
public:
  SmallBLAKE3() : BLAKE3(Storage) {}

private:
  StorageT Storage;
};

// BLAKE3.cpp
#include <llvm-c/blake3.h>
BLAKE3::BLAKE3(StorageT &Storage) {
  static_assert(sizeof(Storage.Impl) >= sizeof(llvm_blake3_hasher));
  static_assert(alignof(Storage.Impl) >= alignof(llvm_blake3_hasher));
  Hasher = reinterpret_cast<llvm_blake3_hasher *>(Storage.Impl);
  llvm_blake3_init(Hasher);
}
```

But we can do one/both of those later if it's important.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121510

STAMPS
actor(@dexonsmith) application(Differential) author(@akyrtzi) herald(H127) herald(H157) herald(H215) herald(H224) herald(H225) herald(H403) herald(H428) herald(H438) herald(H576) herald(H615) herald(H628) herald(H688) herald(H723) herald(H864) monogram(D121510) object-type(DREV) phid(PHID-DREV-2ilvuhmax5o633ygavm7) reviewer(@compnerd) reviewer(@deadalnix) reviewer(@dexonsmith) reviewer(@jdoerfert) reviewer(@lattner) reviewer(@MaskRay) reviewer(@rnk) revision-repository(rG) revision-status(published) subscriber(@dexonsmith) subscriber(@hiraditya) subscriber(@joerg) subscriber(@krytarowski) subscriber(@lebedev.ri) subscriber(@llvm-commits) subscriber(@MaskRay) subscriber(@mgorny) subscriber(@sstefan1) subscriber(@StephenFan) subscriber(@thakis) tag(#all) tag(#llvm) via(web)