[PATCH] D69295: Optimize SHA1 implementation

Wed Oct 23 05:47:08 PDT 2019

kbobyrev added a comment.

In D69295#1718448 <https://reviews.llvm.org/D69295#1718448>, @MaskRay wrote:

> > 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.

Yes, I think putting files into `llvm/benchmarks` would be sensible especially given the lack of other benchmarks right now.

> We probably need more consensus (e.g. send an email to the llvm-dev mailing list, referencing http://lists.llvm.org/pipermail/llvm-dev/2018-August/125173.html @kbobyrev) on how benchmark files should be organized.

I also agree with this. I think `llvm/benchmarks` would be fine for now either way, but with more benchmarks (which hopefully would be there at some point, I had plans of adding some for a while now but I'm not sure when I'll get to that) subdirectories make sense.

Thanks for CCing me!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69295