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

[Eli Friedman]

On Fri, Aug 2, 2013 at 7:35 AM, Stepan Dyatkovskiy <stpworld at narod.ru> wrote:
> Hello everyone,
> there are patches for PR16752.
> Bug desciption (from bugzilla):
> Troubles could be happened due to some assumptions in handleModeAttr
> function (see SemaDeclAttr.cpp).
> For example, it assumes that 32 bit integer is 'int', while it could be 16
> bit only.
> Instead of asking target: 'which type do you want to use for int32_t ?' it
> just hardcodes general opinion. That doesn't looks pretty correct.
> Please consider the next solution:
> 1. In Basic/TargetInfo add getIntTypeByWidth and getRealTypeByWidth virtual
> methods. By default current behaviour could be implemented here.
> 2. Fix handleModeAttr according to new methods in TargetInfo.
> This approach is implemented in the patch attached to this post.
>
> 1st patch extends TargetInfo::IntType (char types) and TargetInfo::RealType
> (NoFloat type).
> 2nd patch extends TargetInfo with getIntTypeByWidth and getRealTypeByWidth,
> it also fixes handleAttrMode function.
> Patches are presented in GIT format and second patch should be applied after
> first one.

   enum RealType {
+    NoFloat = 255,
     Float = 0,
     Double,
-    LongDouble
+    LongDouble,
   };

Unless I'm missing something, there isn't any reason not to make NoFloat 0.

+  /// \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.


Sorry it took a little while to get to reviewing this.

-Eli