[PATCH] D65905: [compiler-rt] Add ConsumeProbability and ConsumeFloatingPoint methods to FDP.

[Max Moroz via Phabricator via llvm-commits]

Dor1s marked 2 inline comments as done.
Dor1s added inline comments.


================
Comment at: include/fuzzer/FuzzedDataProvider.hpp:224
+    constexpr T zero(.0);
+    if ((min >= zero && max >= zero) || (min <= zero && max <= zero)) {
+      // Use range-based implementation, as |min| and |max| have the same
----------------
morehouse wrote:
> Nit:  `min <= max`, so this is equivalent to `min >= 0 || max <= 0`.
Oh, this is smart!


================
Comment at: include/fuzzer/FuzzedDataProvider.hpp:234
+    T positive = max * ConsumeProbability<T>();
+    return min + max;
+  }
----------------
morehouse wrote:
> This seems like it might favor the middle part of the range over others.  Probably not a big issue for fuzzing, but might be worth thinking about.
Great point! Some other methods here explicitly say in the comments that the results distribution is not uniform, so yeah it might be not a big deal, but your comments made me think more and now I have a slightly different implementation in mind. Will upload it tomorrow. Thanks, Matt!


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D65905