Page MenuHomePhabricator

[compiler-rt] Add ConsumeProbability and ConsumeFloatingPoint methods to FDP.
ClosedPublic

Authored by Dor1s on Aug 7 2019, 2:16 PM.

Details

Summary

Also slightly cleaned up the comments and changed the header's extension
back to .h as per comments on https://reviews.llvm.org/D65812.

New methods added:

  • ConsumeProbability returns [0.0, 1.0] by consuming an unsigned integer value from the input data and dividing that value by the integer's max value.
  • ConsumeFloatingPointInRange returns a floating point value in the given range. Relies on ConsumeProbability method. This method does not have the limitation of std::uniform_real_distribution that requires the given range to be <= the floating point type's max. If the range is too large, this implementation will additionally call ConsumeBool to decide whether the result will be in the first or the second half of the range.
  • ConsumeFloatingPoint returns a floating point value in the range [std::numeric_limits<T>::lowest(), std::numeric_limits<T>::min()].

Tested on Linux, Mac, Windows.

Diff Detail

Repository
rL LLVM

Event Timeline

Dor1s created this revision.Aug 7 2019, 2:16 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptAug 7 2019, 2:16 PM
Herald added subscribers: llvm-commits, Restricted Project, delcypher, dberris. · View Herald Transcript
Dor1s updated this revision to Diff 213999.Aug 7 2019, 2:18 PM

Add parens to the condition for better readability

Dor1s edited the summary of this revision. (Show Details)Aug 7 2019, 2:35 PM
Dor1s added a reviewer: morehouse.
Dor1s edited the summary of this revision. (Show Details)
Dor1s edited the summary of this revision. (Show Details)Aug 7 2019, 2:39 PM

@morehouse Ready for review, PTAL once you get a chance :)

morehouse accepted this revision.Aug 7 2019, 4:14 PM
morehouse added inline comments.
include/fuzzer/FuzzedDataProvider.hpp
224 ↗(On Diff #213999)

Nit: min <= max, so this is equivalent to min >= 0 || max <= 0.

234 ↗(On Diff #213999)

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.

This revision is now accepted and ready to land.Aug 7 2019, 4:14 PM
Dor1s marked 2 inline comments as done.Aug 7 2019, 9:15 PM
Dor1s added inline comments.
include/fuzzer/FuzzedDataProvider.hpp
224 ↗(On Diff #213999)

Oh, this is smart!

234 ↗(On Diff #213999)

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!

Dor1s updated this revision to Diff 214162.Aug 8 2019, 8:33 AM
Dor1s edited the summary of this revision. (Show Details)

Re-write ConsumeFloatingPointInRange, change extension back to .h

Dor1s marked 2 inline comments as done.Aug 8 2019, 8:33 AM

Re-written ConsumeFloatingPointInRange in another fashion. Will describe the new logic in the description.

Dor1s edited the summary of this revision. (Show Details)Aug 8 2019, 8:37 AM

@morehouse the CL got a little messy because of the renaming, but I'd appreciate if you could take a look at the new version of ConsumeFloatingPointInRange, it's ~20 lines. Thanks!

morehouse accepted this revision.Aug 8 2019, 12:42 PM
morehouse added inline comments.
include/fuzzer/FuzzedDataProvider.h
229 ↗(On Diff #214162)

Nice!

This revision was automatically updated to reflect the committed changes.