[libc-commits] [PATCH] D93210: [libc] revamp memory function benchmark

[Clement Courbet via Phabricator via libc-commits]

courbet accepted this revision.
courbet added a comment.
This revision is now accepted and ready to land.

Only cosmetic comments.



================
Comment at: libc/benchmarks/LibcMemoryBenchmark.h:41
+  // See 'SweepModeMaxSize' and 'SizeDistributionName' below.
+  bool IsSweepMode = false;
+
----------------
Make this a method ?

```
bool IsSweepMode() {
  return SweepModeMaxSize > 0;
}
```


================
Comment at: libc/benchmarks/LibcMemoryBenchmarkMain.cpp:74
+struct ParameterType {
+  unsigned Offset : 16; // max : 16 KiB - 1
+  unsigned Size : 16;   // max : 16 KiB - 1
----------------
`OffsetBytes` ?


================
Comment at: libc/benchmarks/LibcMemoryBenchmarkMain.cpp:75
+  unsigned Offset : 16; // max : 16 KiB - 1
+  unsigned Size : 16;   // max : 16 KiB - 1
+};
----------------
`SizeBytes` ?


================
Comment at: libc/benchmarks/LibcMemoryBenchmarkMain.cpp:106
+    return [this](ParameterType P) {
+      __llvm_libc::memset(DstBuffer + P.Offset, P.Offset % 0xFF, P.Size);
+      return DstBuffer + P.Offset;
----------------
did you mean `&` ?


================
Comment at: libc/benchmarks/LibcMemoryBenchmarkMain.cpp:149
+size_t getL1DataCacheSize() {
+  const std::vector<CacheInfo> CacheInfos = HostState::get().Caches;
+  const auto IsL1DataCache = [](const CacheInfo &CI) {
----------------
why not `const &` ?


================
Comment at: libc/benchmarks/README.md:87
+
+> Note: `Python 2` [being deprecated](https://www.python.org/doc/sunset-python-2/) it is advised to used `Python 3`.
+
----------------
I think you can drop this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93210