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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D140636#4016185 <https://reviews.llvm.org/D140636#4016185>, @craig.topper wrote:

> In D140636#4016133 <https://reviews.llvm.org/D140636#4016133>, @barannikov88 wrote:
>
>> In D140636#4016127 <https://reviews.llvm.org/D140636#4016127>, @lebedev.ri wrote:
>>
>>> Let me add some tests here.
>>
>> In case that helps, I've tried a few bitwidth variants.
>> With LeadingZeros = 0, I get different magics iff the divisor has LSB and MSB ones and all other bits zeros. (I.e. 0b1000...0001 for any bitwidth.)
>> Example: APInt(8, 0x81).
>>
>> With LeadingZeros != 0, the old algorithm asserts iff the divisor is a certain power of two. Examples:
>>  D = APInt(8, 0x80) and LeadingZeros = 1
>>  D = APInt(8, 0x40) and LeadingZeros = 2.
>> The fixed version does not crash and returns what seems to be correct values (can that be enough for a unittest?)
>
> I'm not sure I expect the algorithm to work if the divisor does not have at least as many leading zeros as the dividend. The AllOnes value the algorithm creates should be >= D. Norrmally when LeadingZeros is used both the dividend and the divisor have been shifted right by that many bits.

ACK. It's not a nice API design, but you can't just pass random `LeadingZeros` there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140636