[PATCH] D69295: Optimize SHA1 implementation

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 22 22:53:46 PDT 2019


MaskRay added a comment.

> Add inline to the helper functions because gcc-9 won't inline all of them without the hint. I've avoided __attribute__((always_inline)) because gcc and clang will inline without it, and improves compatibility.

Is it a `-DCMAKE_BUILD_TYPE=Release` build? I will be pretty surprised if gcc/clang -O3 does not inline these functions.

The code change looks good, except that the `llvm/benchmarks/` directory is underused and you are adding more files. We probably need more consensus (e.g. send an email to the llvm-dev mailing list) on how benchmark files should be organized.



================
Comment at: llvm/unittests/Support/raw_sha1_ostream_test.cpp:48
+  SHA1 sha1;
+  std::string Input = "123456789012345678901234567890";
+  ASSERT_EQ(Input.size(), 30UL);
----------------
This can be a `const char []` or StringRef, can't it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69295





More information about the llvm-commits mailing list