[PATCH] D140636: [Support] Fix what I think is an off by 1 bug in UnsignedDivisionByConstantInfo.

[Sergei Barannikov via Phabricator via llvm-commits]

barannikov88 added a comment.

@lebedev.ri 
Thank you for volunteering adding the tests. Rare developer is willing to do so.

I have some concerns about the quality of the tests though.
The tests do not actually test the algorithm in question. They test a division algorithm
using the magic numbers, which is another, and huge, failure point.
Because of this failure point, the tests do not catch a real bug,
(e.g. the generated magic numbers are invalid for APInt(8, 0x81)).
Also, the tests should also cover non-zero LeadingZeros case.

Iterating over the whole search space may be nice. However, the most interesting cases of 32- and 64-bit divisor are not covered
(and can't be covered for performance reasons).

Would you mind replacing them with a few targeted tests that only call the 'get' method and check the returned result?
If you find this request inappropriate given the time you've already spent, that's absolutely OK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140636