<div dir="ltr">On Fri, Sep 6, 2013 at 2:53 AM, Stepan Dyatkovskiy <span dir="ltr"><<a href="mailto:stpworld@narod.ru" target="_blank">stpworld@narod.ru</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Eli OK.<br>
<br>
You mean this one:<div class="im"><br>
case 128:<br>
- if (&getLongDoubleFormat() == &llvm::APFloat::<u></u>PPCDoubleDouble)<br></div>
+ if (&getLongDoubleFormat() == &llvm::APFloat::<u></u>PPCDoubleDouble ||<br>
+ &getLongDoubleFormat() == &llvm::APFloat::IEEEquad)<br>
return LongDouble;<br>
break;<br>
}<br>
If yes - can I fix it?<br></blockquote><div><br></div><div>Yes.<br><br>-Eli<br> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Eli Friedman wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
On Thu, Sep 5, 2013 at 4:23 AM, Stepan Dyatkovskiy <<a href="mailto:stpworld@narod.ru" target="_blank">stpworld@narod.ru</a><br></div><div><div class="h5">
<mailto:<a href="mailto:stpworld@narod.ru" target="_blank">stpworld@narod.ru</a>>> wrote:<br>
<br>
Author: dyatkovskiy<br>
Date: Thu Sep 5 06:23:21 2013<br>
New Revision: 190044<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=190044&view=rev" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project?rev=190044&view=rev</a><br>
Log:<br>
Add new methods for TargetInfo:<br>
getRealTypeByWidth and getIntTypeByWidth<br>
for ASTContext names are almost same(invokes new methods from<br>
TargetInfo):<br>
getIntTypeForBitwidth and getRealTypeForBitwidth.<br>
<br>
As first commit for PR16752 fix: 'mode' attribute for unusual<br>
targets doesn't work properly<br>
Description:<br>
Troubles could be happened due to some assumptions in handleModeAttr<br>
function (see SemaDeclAttr.cpp).<br>
For example, it assumes that 32 bit integer is 'int', while it could<br>
be 16 bit only.<br>
Instead of asking target: 'which type do you want to use for int32_t<br>
?' it just hardcodes general opinion. That doesn't looks pretty correct.<br>
Please consider the next solution:<br>
1. In Basic/TargetInfo add getIntTypeByWidth and getRealTypeByWidth<br>
virtual methods. By default current behaviour could be implemented here.<br>
2. Fix handleModeAttr according to new methods in TargetInfo.<br>
This approach is implemented in the patch attached to this post.<br>
<br>
Fixes:<br>
1st Commit (Current): Add new methods for TargetInfo:<br>
getRealTypeByWidth and getIntTypeByWidth<br>
for ASTContext names are almost same(invokes new methods from<br>
TargetInfo):<br>
getIntTypeForBitwidth and getRealTypeForBitwidth<br>
<br>
2nd Commit (Next): Fix SemaDeclAttr, handleModeAttr function.<br>
<br>
<br>
Modified:<br>
cfe/trunk/include/clang/AST/<u></u>ASTContext.h<br>
cfe/trunk/include/clang/Basic/<u></u>TargetInfo.h<br>
cfe/trunk/lib/AST/ASTContext.<u></u>cpp<br>
cfe/trunk/lib/Basic/<u></u>TargetInfo.cpp<br>
cfe/trunk/lib/Frontend/<u></u>InitPreprocessor.cpp<br>
<br>
Modified: cfe/trunk/include/clang/AST/<u></u>ASTContext.h<br>
URL:<br>
<a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=190044&r1=190043&r2=190044&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/cfe/trunk/include/<u></u>clang/AST/ASTContext.h?rev=<u></u>190044&r1=190043&r2=190044&<u></u>view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- cfe/trunk/include/clang/AST/<u></u>ASTContext.h (original)<br>
+++ cfe/trunk/include/clang/AST/<u></u>ASTContext.h Thu Sep 5 06:23:21 2013<br>
@@ -480,6 +480,17 @@ public:<br>
<br>
const TargetInfo &getTargetInfo() const { return *Target; }<br>
<br>
+ /// getIntTypeForBitwidth -<br>
+ /// sets integer QualTy according to specified details:<br>
+ /// bitwidth, signed/unsigned.<br>
+ /// Returns empty type if there is no appropriate target types.<br>
+ QualType getIntTypeForBitwidth(unsigned DestWidth,<br>
+ unsigned Signed) const;<br>
+ /// getRealTypeForBitwidth -<br>
+ /// sets floating point QualTy according to specified bitwidth.<br>
+ /// Returns empty type if there is no appropriate target types.<br>
+ QualType getRealTypeForBitwidth(<u></u>unsigned DestWidth) const;<br>
+<br>
bool AtomicUsesUnsupportedLibcall(<u></u>const AtomicExpr *E) const;<br>
<br>
const LangOptions& getLangOpts() const { return LangOpts; }<br>
<br>
Modified: cfe/trunk/include/clang/Basic/<u></u>TargetInfo.h<br>
URL:<br>
<a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TargetInfo.h?rev=190044&r1=190043&r2=190044&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/cfe/trunk/include/<u></u>clang/Basic/TargetInfo.h?rev=<u></u>190044&r1=190043&r2=190044&<u></u>view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- cfe/trunk/include/clang/Basic/<u></u>TargetInfo.h (original)<br>
+++ cfe/trunk/include/clang/Basic/<u></u>TargetInfo.h Thu Sep 5 06:23:21 2013<br>
@@ -112,6 +112,8 @@ public:<br>
///===---- Target Data Type Query Methods<br>
------------------------------<u></u>-===//<br>
enum IntType {<br>
NoInt = 0,<br>
+ SignedChar,<br>
+ UnsignedChar,<br>
SignedShort,<br>
UnsignedShort,<br>
SignedInt,<br>
@@ -123,6 +125,7 @@ public:<br>
};<br>
<br>
enum RealType {<br>
+ NoFloat = 255,<br>
Float = 0,<br>
Double,<br>
LongDouble<br>
@@ -220,6 +223,12 @@ public:<br>
/// For example, SignedInt -> getIntWidth().<br>
unsigned getTypeWidth(IntType T) const;<br>
<br>
+ /// \brief Return integer type with specified width.<br>
+ IntType getIntTypeByWidth(unsigned BitWidth, bool IsSigned) const;<br>
+<br>
+ /// \brief Return floating point type with specified width.<br>
+ RealType getRealTypeByWidth(unsigned BitWidth) const;<br>
+<br>
/// \brief Return the alignment (in bits) of the specified<br>
integer type enum.<br>
///<br>
/// For example, SignedInt -> getIntAlign().<br>
<br>
Modified: cfe/trunk/lib/AST/ASTContext.<u></u>cpp<br>
URL:<br>
<a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=190044&r1=190043&r2=190044&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/cfe/trunk/lib/AST/<u></u>ASTContext.cpp?rev=190044&r1=<u></u>190043&r2=190044&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- cfe/trunk/lib/AST/ASTContext.<u></u>cpp (original)<br>
+++ cfe/trunk/lib/AST/ASTContext.<u></u>cpp Thu Sep 5 06:23:21 2013<br>
@@ -6314,6 +6314,8 @@ ASTContext::<u></u>getSubstTemplateTemplateParm<br>
CanQualType ASTContext::getFromTargetType(<u></u>unsigned Type) const {<br>
switch (Type) {<br>
case TargetInfo::NoInt: return CanQualType();<br>
+ case TargetInfo::SignedChar: return SignedCharTy;<br>
+ case TargetInfo::UnsignedChar: return UnsignedCharTy;<br>
case TargetInfo::SignedShort: return ShortTy;<br>
case TargetInfo::UnsignedShort: return UnsignedShortTy;<br>
case TargetInfo::SignedInt: return IntTy;<br>
@@ -7990,6 +7992,38 @@ size_t ASTContext::<u></u>getSideTableAllocated<br>
llvm::capacity_in_bytes(<u></u>ClassScopeSpecializationPatter<u></u>n);<br>
}<br>
<br>
+/// getIntTypeForBitwidth -<br>
+/// sets integer QualTy according to specified details:<br>
+/// bitwidth, signed/unsigned.<br>
+/// Returns empty type if there is no appropriate target types.<br>
+QualType ASTContext::<u></u>getIntTypeForBitwidth(unsigned DestWidth,<br>
+ unsigned Signed) const {<br>
+ TargetInfo::IntType Ty =<br>
getTargetInfo().<u></u>getIntTypeByWidth(DestWidth, Signed);<br>
+ CanQualType QualTy = getFromTargetType(Ty);<br>
+ if (!QualTy && DestWidth == 128)<br>
+ return Signed ? Int128Ty : UnsignedInt128Ty;<br>
+ return QualTy;<br>
+}<br>
+<br>
+/// getRealTypeForBitwidth -<br>
+/// sets floating point QualTy according to specified bitwidth.<br>
+/// Returns empty type if there is no appropriate target types.<br>
+QualType ASTContext::<u></u>getRealTypeForBitwidth(<u></u>unsigned DestWidth) const {<br>
+ TargetInfo::RealType Ty =<br>
getTargetInfo().<u></u>getRealTypeByWidth(DestWidth);<br>
+ switch (Ty) {<br>
+ case TargetInfo::Float:<br>
+ return FloatTy;<br>
+ case TargetInfo::Double:<br>
+ return DoubleTy;<br>
+ case TargetInfo::LongDouble:<br>
+ return LongDoubleTy;<br>
+ case TargetInfo::NoFloat:<br>
+ return QualType();<br>
+ }<br>
+<br>
+ llvm_unreachable("Unhandled TargetInfo::RealType value");<br>
+}<br>
+<br>
void ASTContext::setManglingNumber(<u></u>const NamedDecl *ND, unsigned<br>
Number) {<br>
if (Number > 1)<br>
MangleNumbers[ND] = Number;<br>
<br>
Modified: cfe/trunk/lib/Basic/<u></u>TargetInfo.cpp<br>
URL:<br>
<a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/TargetInfo.cpp?rev=190044&r1=190043&r2=190044&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/cfe/trunk/lib/Basic/<u></u>TargetInfo.cpp?rev=190044&r1=<u></u>190043&r2=190044&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- cfe/trunk/lib/Basic/<u></u>TargetInfo.cpp (original)<br>
+++ cfe/trunk/lib/Basic/<u></u>TargetInfo.cpp Thu Sep 5 06:23:21 2013<br>
@@ -102,6 +102,8 @@ TargetInfo::~TargetInfo() {}<br>
const char *TargetInfo::getTypeName(<u></u>IntType T) {<br>
switch (T) {<br>
default: llvm_unreachable("not an integer!");<br>
+ case SignedChar: return "char";<br>
+ case UnsignedChar: return "unsigned char";<br>
case SignedShort: return "short";<br>
case UnsignedShort: return "unsigned short";<br>
case SignedInt: return "int";<br>
@@ -118,10 +120,12 @@ const char *TargetInfo::getTypeName(IntT<br>
const char *TargetInfo::<u></u>getTypeConstantSuffix(IntType T) {<br>
switch (T) {<br>
default: llvm_unreachable("not an integer!");<br>
+ case SignedChar:<br>
case SignedShort:<br>
case SignedInt: return "";<br>
case SignedLong: return "L";<br>
case SignedLongLong: return "LL";<br>
+ case UnsignedChar:<br>
case UnsignedShort:<br>
case UnsignedInt: return "U";<br>
case UnsignedLong: return "UL";<br>
@@ -134,6 +138,8 @@ const char *TargetInfo::getTypeConstantS<br>
unsigned TargetInfo::getTypeWidth(<u></u>IntType T) const {<br>
switch (T) {<br>
default: llvm_unreachable("not an integer!");<br>
+ case SignedChar:<br>
+ case UnsignedChar: return getCharWidth();<br>
case SignedShort:<br>
case UnsignedShort: return getShortWidth();<br>
case SignedInt:<br>
@@ -145,11 +151,48 @@ unsigned TargetInfo::getTypeWidth(<u></u>IntTyp<br>
};<br>
}<br>
<br>
+TargetInfo::IntType TargetInfo::getIntTypeByWidth(<br>
+ unsigned BitWidth, bool IsSigned) const {<br>
+ if (getCharWidth() == BitWidth)<br>
+ return IsSigned ? SignedChar : UnsignedChar;<br>
+ if (getShortWidth() == BitWidth)<br>
+ return IsSigned ? SignedShort : UnsignedShort;<br>
+ if (getIntWidth() == BitWidth)<br>
+ return IsSigned ? SignedInt : UnsignedInt;<br>
+ if (getLongWidth() == BitWidth)<br>
+ return IsSigned ? SignedLong : UnsignedLong;<br>
+ if (getLongLongWidth() == BitWidth)<br>
+ return IsSigned ? SignedLongLong : UnsignedLongLong;<br>
+ return NoInt;<br>
+}<br>
+<br>
+TargetInfo::RealType TargetInfo::<u></u>getRealTypeByWidth(unsigned<br>
BitWidth) const {<br>
+ if (getFloatWidth() == BitWidth)<br>
+ return Float;<br>
+ if (getDoubleWidth() == BitWidth)<br>
+ return Double;<br>
+<br>
+ switch (BitWidth) {<br>
+ case 96:<br>
+ if (&getLongDoubleFormat() == &llvm::APFloat::<u></u>x87DoubleExtended)<br>
+ return LongDouble;<br>
+ break;<br>
+ case 128:<br>
+ if (&getLongDoubleFormat() == &llvm::APFloat::<u></u>PPCDoubleDouble)<br>
+ return LongDouble;<br>
<br>
<br>
You probably need to check for IEEEquad here as well.<br>
<br>
I won't ask you to revert this, since the patch is correct otherwise,<br>
but in the future, please don't commit patches while they are in the<br>
middle of being reviewed.<br>
<br>
-Eli<br>
</div></div></blockquote>
<br>
</blockquote></div><br></div></div>