[llvm-commits] [llvm] r60275 - in /llvm/trunk: lib/Transforms/Scalar/InstructionCombining.cpp test/Transforms/InstCombine/apint-sub.ll test/Transforms/InstCombine/sdiv-1.ll test/Transforms/InstCombine/sub.ll

[Bill Wendling]

On Dec 1, 2008, at 12:03 AM, Duncan Sands wrote:

> Hi Bill,
>
>> I think that negate only overflows on minint, instead of:
>
> if X has any bit zero, then it can't be minint.  So maybe
> this transform can be done for non-constants by querying some
> "what bits are set" mechanism.
>
I'm not sure how that would work.

>>> +    // -X/C -> X/-C, if and only if negation doesn't overflow.
>
> Also, I'm a bit confused about the tests.  As far as I can see,
> what you care about is whether X overflows, not whether C overflows,
> but you seem to test the later.  Clearly, if (-X) overflows then
> the transform is wrong.  Now suppose (-C) overflows.  Then C is
> minint, so -C = minint also.  Thus (-X)/C and X/(-C) are both
> zero (i.e. the transform is correct) unless -X or X is minint,
> which is the same as X = minint.  In otherwords, again the transform
> is correct unless -X overflows.  Thus checking whether -X overflows
> catches all cases when the transform is incorrect.
>
Actually, I test whether either -X or -C overflows. Testing whether -C  
overflows might be overkill because we use APInt and friends. I was  
dubious about this whole transform in the PR. The changes I did here  
really only serve to stop us from doing this transformation when X is  
non-constant.

-bw