Eli OK.

You mean this one:
   case 128:
-    if (&getLongDoubleFormat() == &llvm::APFloat::PPCDoubleDouble)
+    if (&getLongDoubleFormat() == &llvm::APFloat::PPCDoubleDouble ||
+        &getLongDoubleFormat() == &llvm::APFloat::IEEEquad)
       return LongDouble;
     break;
   }
If yes - can I fix it?

Eli Friedman wrote:
On Thu, Sep 5, 2013 at 4:23 AM, Stepan Dyatkovskiy <[email protected]
<mailto:[email protected]>> wrote:

    Author: dyatkovskiy
    Date: Thu Sep  5 06:23:21 2013
    New Revision: 190044

    URL: http://llvm.org/viewvc/llvm-project?rev=190044&view=rev
    Log:
    Add new methods for TargetInfo:
          getRealTypeByWidth and getIntTypeByWidth
       for ASTContext names are almost same(invokes new methods from
    TargetInfo):
          getIntTypeForBitwidth and getRealTypeForBitwidth.

    As first commit for PR16752 fix: 'mode' attribute for unusual
    targets doesn't work properly
    Description:
    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.

    Fixes:
    1st Commit (Current): Add new methods for TargetInfo:
          getRealTypeByWidth and getIntTypeByWidth
       for ASTContext names are almost same(invokes new methods from
    TargetInfo):
          getIntTypeForBitwidth and getRealTypeForBitwidth

    2nd Commit (Next): Fix SemaDeclAttr, handleModeAttr function.


    Modified:
         cfe/trunk/include/clang/AST/ASTContext.h
         cfe/trunk/include/clang/Basic/TargetInfo.h
         cfe/trunk/lib/AST/ASTContext.cpp
         cfe/trunk/lib/Basic/TargetInfo.cpp
         cfe/trunk/lib/Frontend/InitPreprocessor.cpp

    Modified: cfe/trunk/include/clang/AST/ASTContext.h
    URL:
    
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=190044&r1=190043&r2=190044&view=diff
    
==============================================================================
    --- cfe/trunk/include/clang/AST/ASTContext.h (original)
    +++ cfe/trunk/include/clang/AST/ASTContext.h Thu Sep  5 06:23:21 2013
    @@ -480,6 +480,17 @@ public:

        const TargetInfo &getTargetInfo() const { return *Target; }

    +  /// getIntTypeForBitwidth -
    +  /// sets integer QualTy according to specified details:
    +  /// bitwidth, signed/unsigned.
    +  /// Returns empty type if there is no appropriate target types.
    +  QualType getIntTypeForBitwidth(unsigned DestWidth,
    +                                 unsigned Signed) const;
    +  /// getRealTypeForBitwidth -
    +  /// sets floating point QualTy according to specified bitwidth.
    +  /// Returns empty type if there is no appropriate target types.
    +  QualType getRealTypeForBitwidth(unsigned DestWidth) const;
    +
        bool AtomicUsesUnsupportedLibcall(const AtomicExpr *E) const;

        const LangOptions& getLangOpts() const { return LangOpts; }

    Modified: cfe/trunk/include/clang/Basic/TargetInfo.h
    URL:
    
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TargetInfo.h?rev=190044&r1=190043&r2=190044&view=diff
    
==============================================================================
    --- cfe/trunk/include/clang/Basic/TargetInfo.h (original)
    +++ cfe/trunk/include/clang/Basic/TargetInfo.h Thu Sep  5 06:23:21 2013
    @@ -112,6 +112,8 @@ public:
        ///===---- Target Data Type Query Methods
    -------------------------------===//
        enum IntType {
          NoInt = 0,
    +    SignedChar,
    +    UnsignedChar,
          SignedShort,
          UnsignedShort,
          SignedInt,
    @@ -123,6 +125,7 @@ public:
        };

        enum RealType {
    +    NoFloat = 255,
          Float = 0,
          Double,
          LongDouble
    @@ -220,6 +223,12 @@ public:
        /// For example, SignedInt -> getIntWidth().
        unsigned getTypeWidth(IntType T) const;

    +  /// \brief Return integer type with specified width.
    +  IntType getIntTypeByWidth(unsigned BitWidth, bool IsSigned) const;
    +
    +  /// \brief Return floating point type with specified width.
    +  RealType getRealTypeByWidth(unsigned BitWidth) const;
    +
        /// \brief Return the alignment (in bits) of the specified
    integer type enum.
        ///
        /// For example, SignedInt -> getIntAlign().

    Modified: cfe/trunk/lib/AST/ASTContext.cpp
    URL:
    
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=190044&r1=190043&r2=190044&view=diff
    
==============================================================================
    --- cfe/trunk/lib/AST/ASTContext.cpp (original)
    +++ cfe/trunk/lib/AST/ASTContext.cpp Thu Sep  5 06:23:21 2013
    @@ -6314,6 +6314,8 @@ ASTContext::getSubstTemplateTemplateParm
      CanQualType ASTContext::getFromTargetType(unsigned Type) const {
        switch (Type) {
        case TargetInfo::NoInt: return CanQualType();
    +  case TargetInfo::SignedChar: return SignedCharTy;
    +  case TargetInfo::UnsignedChar: return UnsignedCharTy;
        case TargetInfo::SignedShort: return ShortTy;
        case TargetInfo::UnsignedShort: return UnsignedShortTy;
        case TargetInfo::SignedInt: return IntTy;
    @@ -7990,6 +7992,38 @@ size_t ASTContext::getSideTableAllocated
               llvm::capacity_in_bytes(ClassScopeSpecializationPattern);
      }

    +/// getIntTypeForBitwidth -
    +/// sets integer QualTy according to specified details:
    +/// bitwidth, signed/unsigned.
    +/// Returns empty type if there is no appropriate target types.
    +QualType ASTContext::getIntTypeForBitwidth(unsigned DestWidth,
    +                                           unsigned Signed) const {
    +  TargetInfo::IntType Ty =
    getTargetInfo().getIntTypeByWidth(DestWidth, Signed);
    +  CanQualType QualTy = getFromTargetType(Ty);
    +  if (!QualTy && DestWidth == 128)
    +    return Signed ? Int128Ty : UnsignedInt128Ty;
    +  return QualTy;
    +}
    +
    +/// getRealTypeForBitwidth -
    +/// sets floating point QualTy according to specified bitwidth.
    +/// Returns empty type if there is no appropriate target types.
    +QualType ASTContext::getRealTypeForBitwidth(unsigned DestWidth) const {
    +  TargetInfo::RealType Ty =
    getTargetInfo().getRealTypeByWidth(DestWidth);
    +  switch (Ty) {
    +  case TargetInfo::Float:
    +    return FloatTy;
    +  case TargetInfo::Double:
    +    return DoubleTy;
    +  case TargetInfo::LongDouble:
    +    return LongDoubleTy;
    +  case TargetInfo::NoFloat:
    +    return QualType();
    +  }
    +
    +  llvm_unreachable("Unhandled TargetInfo::RealType value");
    +}
    +
      void ASTContext::setManglingNumber(const NamedDecl *ND, unsigned
    Number) {
        if (Number > 1)
          MangleNumbers[ND] = Number;

    Modified: cfe/trunk/lib/Basic/TargetInfo.cpp
    URL:
    
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/TargetInfo.cpp?rev=190044&r1=190043&r2=190044&view=diff
    
==============================================================================
    --- cfe/trunk/lib/Basic/TargetInfo.cpp (original)
    +++ cfe/trunk/lib/Basic/TargetInfo.cpp Thu Sep  5 06:23:21 2013
    @@ -102,6 +102,8 @@ TargetInfo::~TargetInfo() {}
      const char *TargetInfo::getTypeName(IntType T) {
        switch (T) {
        default: llvm_unreachable("not an integer!");
    +  case SignedChar:       return "char";
    +  case UnsignedChar:     return "unsigned char";
        case SignedShort:      return "short";
        case UnsignedShort:    return "unsigned short";
        case SignedInt:        return "int";
    @@ -118,10 +120,12 @@ const char *TargetInfo::getTypeName(IntT
      const char *TargetInfo::getTypeConstantSuffix(IntType T) {
        switch (T) {
        default: llvm_unreachable("not an integer!");
    +  case SignedChar:
        case SignedShort:
        case SignedInt:        return "";
        case SignedLong:       return "L";
        case SignedLongLong:   return "LL";
    +  case UnsignedChar:
        case UnsignedShort:
        case UnsignedInt:      return "U";
        case UnsignedLong:     return "UL";
    @@ -134,6 +138,8 @@ const char *TargetInfo::getTypeConstantS
      unsigned TargetInfo::getTypeWidth(IntType T) const {
        switch (T) {
        default: llvm_unreachable("not an integer!");
    +  case SignedChar:
    +  case UnsignedChar:     return getCharWidth();
        case SignedShort:
        case UnsignedShort:    return getShortWidth();
        case SignedInt:
    @@ -145,11 +151,48 @@ unsigned TargetInfo::getTypeWidth(IntTyp
        };
      }

    +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;
    +}
    +
    +TargetInfo::RealType TargetInfo::getRealTypeByWidth(unsigned
    BitWidth) const {
    +  if (getFloatWidth() == BitWidth)
    +    return Float;
    +  if (getDoubleWidth() == BitWidth)
    +    return Double;
    +
    +  switch (BitWidth) {
    +  case 96:
    +    if (&getLongDoubleFormat() == &llvm::APFloat::x87DoubleExtended)
    +      return LongDouble;
    +    break;
    +  case 128:
    +    if (&getLongDoubleFormat() == &llvm::APFloat::PPCDoubleDouble)
    +      return LongDouble;


You probably need to check for IEEEquad here as well.

I won't ask you to revert this, since the patch is correct otherwise,
but in the future, please don't commit patches while they are in the
middle of being reviewed.

-Eli

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to