[Review]: Fix Integer Division Expansion special case of divide by 1

Michael Ilseman milseman at apple.com
Tue Jul 7 13:32:35 PDT 2015


> On Jul 6, 2015, at 4:43 PM, Aditya Nandakumar <aditya_nandakumar at apple.com> wrote:
> 
> The problem is the check is a special case for handling X/1 - even if the special case is not being used, the expansion into the loop should still produce a valid quotient. It would be hard to test before and after for correctness.
> The problem with the out of tree target is likely in the codegen of the expansion into the loop - and changing the special case of divide by 1 only fixes that case.


I must not understand correctly. It sounds like you are positing the following:
1) There is a logic flaw that this corrects.
2) There is no bit pattern for the inputs/output that demonstrates this flaw.

#1 sounds probable, but I don’t understand how #2 is possible.

>> On Jul 6, 2015, at 4:35 PM, Michael Ilseman <milseman at apple.com> wrote:
>> 
>> Then can you sanitize and commit such a test case?
>> 
>>> On Jul 6, 2015, at 4:26 PM, Aditya Nandakumar <aditya_nandakumar at apple.com> wrote:
>>> 
>>> I don’t have a test case before and after for in-tree target but it fixed test for out of tree target.
>>> The logic is what I am pointing out is wrong - we need to check if CTLZ(divisor) == Num_bits - 1 rather than CTLZ(divisor) - CTLZ(dividend).
>>>> On Jul 6, 2015, at 2:29 PM, Michael Ilseman <milseman at apple.com> wrote:
>>>> 
>>>> Do you have a test case that demonstrates the failure before-hand, and the fix after? 
>>>> 
>>>>> On Jul 2, 2015, at 12:26 PM, Aditya Nandakumar <aditya_nandakumar at apple.com> wrote:
>>>>> 
>>>>> <DiffIntUnsignedDivision.patch>
>>>> 
>>> 
>> 
> 





More information about the llvm-commits mailing list