[cfe-commits] r172570 - /cfe/trunk/lib/CodeGen/CGExpr.cpp

[Dmitri Gribenko]

On Fri, Jan 18, 2013 at 1:30 AM, Richard Smith <richard at metafoo.co.uk> wrote:
> On Thu, Jan 17, 2013 at 10:06 AM, Dmitri Gribenko <gribozavr at gmail.com> wrote:
>> Hello David,
>>
>> On Wed, Jan 16, 2013 at 1:13 AM, David Greene <greened at obbligato.org> wrote:
>>> Author: greened
>>> Date: Tue Jan 15 17:13:47 2013
>>> New Revision: 172570
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=172570&view=rev
>>> Log:
>>> Avoid unsigned Compare to int
>>>
>>> Cast arithmetic results to avoid comparison of an unsigned to an int.
>>>
>>> Modified:
>>>     cfe/trunk/lib/CodeGen/CGExpr.cpp
>>>
>>> Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=172570&r1=172569&r2=172570&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
>>> +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Tue Jan 15 17:13:47 2013
>>> @@ -1191,7 +1191,7 @@
>>>    cast<llvm::LoadInst>(Val)->setAlignment(Info.StorageAlignment);
>>>
>>>    if (Info.IsSigned) {
>>> -    assert((Info.Offset + Info.Size) <= Info.StorageSize);
>>> +    assert(static_cast<unsigned>(Info.Offset + Info.Size) <= Info.StorageSize);
>>>      unsigned HighBits = Info.StorageSize - Info.Offset - Info.Size;
>>>      if (HighBits)
>>>        Val = Builder.CreateShl(Val, HighBits, "bf.shl");
>>
>> Which version of gcc do you use?
>>
>> I tested gcc 4.4, 4.6, 4.7 with -W -Wall -Wextra on
>>
>> int f(unsigned a, unsigned b, unsigned c) {
>>   if (a + b <= c)
>>     return 42;
>>   return 0;
>> }
>>
>> and I don't see any warnings.
>
> This is due to GCC's misimplementation of bitfields (they implement a
> resolution to a WG14 defect report which the C committee decided not
> to ratify). They treat the bitwidth as being part of the type, and
> then the usual arithmetic conversions for 'unsigned:16 + unsigned:15'
> give 'int' as the type of the LHS of the <=.

I see, thank you.  With a test adjusted according to your explanation
I do see warnings.

Dmitri

-- 
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/