PR16752[PATCH]: 'mode' attribute for unusual targets doesn't work properly.

[Eli Friedman]

On Sun, Aug 25, 2013 at 9:34 AM, Stepan Dyatkovskiy <stpworld at narod.ru>wrote:

> Hi Eli,
> Thanks for review!
>
>
> >
>
>>     enum RealType {
>> +    NoFloat = 255,
>>       Float = 0,
>>       Double,
>> -    LongDouble
>> +    LongDouble,
>>     };
>>
>> Unless I'm missing something, there isn't any reason not to make NoFloat
>> 0.
>>
> Yes, there is. Since there types could be encoded, and Float is supposed
> to be encoded as 0. It couses several crashes:
>     Clang :: CodeGenObjC/fpret.m
>     Clang :: CodeGenObjC/metadata-symbols-**64.m
>     Clang :: Sema/attr-mode.c
>
> Oh, okay, that's fine.


>
>
>> +  /// \brief Return integer type with specified width.
>> +  virtual IntType getIntTypeByWidth(unsigned BitWidth, bool IsSigned)
>> const;
>> +
>> +  /// \brief Return floating point type with specified width.
>> +  virtual RealType getRealTypeByWidth(unsigned BitWidth) const;
>>
>> Instead of making these virtual, would it be possible to make these
>> figure out the appropriate types based on the widths etc. we already
>> compute?  e.g. for the integer types, something like so:
>>
>> TargetInfo::IntType TargetInfo::getIntTypeByWidth(
>>      unsigned BitWidth, bool IsSigned) const {
>>    if (getCharWidth() == BitWidth)
>>      return IsSigned ? SignedChar : UnsignedChar;
>>    if (getShortWidth() == BitWidth)
>>      return IsSigned ? SignedShort : UnsignedShort;
>>    if (getIntWidth() == BitWidth)
>>      return IsSigned ? SignedInt : UnsignedInt;
>>    if (getLongWidth() == BitWidth)
>>      return IsSigned ? SignedLong : UnsignedLong;
>>    if (getLongLongWidth() == BitWidth)
>>      return IsSigned ? SignedLongLong : UnsignedLongLong;
>>    return NoInt;
>> }
>>
>> I'd prefer to make things as simple as possible for people adding new
>> targets.
>>
> OK. I did it. The reason was to keep current implementation but in better
> form, so everybody could customize it. Implementation you proposed could
> work differently, though I can watch for buildbots after commit and reject
> it if something went wrong.
>
>
>  Sorry it took a little while to get to reviewing this.
>>
> Sorry me to. I was on vocation :-)
>
>
-    LongDouble
+    LongDouble,
   };

I don't recall whether all the compilers we care about accept this without
complaining; please leave out the extra comma.

Otherwise, LGTM.

-Eli
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130829/ee4d764f/attachment.html>