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

Stepan Dyatkovskiy stpworld at narod.ru
Sun Aug 25 09:34:52 PDT 2013


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

>
> +  /// \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 :-)

-Stepan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: ModeAttr-2013-08-25-git-all.patch
Type: text/x-diff
Size: 10892 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130825/207f5169/attachment.patch>


More information about the cfe-commits mailing list