[llvm-commits] [llvm] r168181 - in /llvm/trunk: lib/Transforms/InstCombine/InstCombineCompares.cpp test/Transforms/InstCombine/icmp.ll

[Duncan Sands]

On 22/11/12 04:24, NAKAMURA Takumi wrote:
> Pawel, pull part of r168196 into release_32, or warnings could be seen.

I don't think having the build be warning free is part of the release criteria.
That said, I've got nothing against pulling r168196, which is risk free.

Ciao, Duncan.

>
> ...Takumi
>
> 2012/11/17 Duncan Sands <baldrick at free.fr>:
>> Hi Chandler,
>>
>>
>>> Maybe a comment about why this set of tests is the correct way to
>>> determine Y and Z?
>>>
>>>> +      Value *Y, *Z;
>>>> +      if (A == C) {
>>>> +        Y = B;
>>>> +        Z = D;
>>>> +      } else if (A == D) {
>>>> +        Y = B;
>>>> +        Z = C;
>>>> +      } else if (B == C) {
>>>> +        Y = A;
>>>> +        Z = D;
>>>> +      } else if (B == D) {
>>>> +        Y = A;
>>>> +        Z = C;
>>>> +      }
>>>
>>>
>>> Maybe add an:
>>>
>>> } else {
>>>     llvm_unreachable(...);
>>> }
>>>
>>> so that we catch a bug of this form with something cheaper than Valgrind.
>>
>>
>> thanks for the suggestion.  I've added some comments in commit 168196 which
>> hopefully make it clear.
>>
>> Ciao, Duncan.
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits