On Sun, Aug 25, 2013 at 9:34 AM, Stepan Dyatkovskiy <[email protected]>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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
