<div dir="ltr">On Sun, Aug 25, 2013 at 9:34 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:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Hi Eli,<br>
Thanks for review!<div class="im"><br>
<br>
><br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
    enum RealType {<br>
+    NoFloat = 255,<br>
      Float = 0,<br>
      Double,<br>
-    LongDouble<br>
+    LongDouble,<br>
    };<br>
<br>
Unless I'm missing something, there isn't any reason not to make NoFloat 0.<br>
</blockquote></div>
Yes, there is. Since there types could be encoded, and Float is supposed to be encoded as 0. It couses several crashes:<br>
    Clang :: CodeGenObjC/fpret.m<br>
    Clang :: CodeGenObjC/metadata-symbols-<u></u>64.m<br>
    Clang :: Sema/attr-mode.c<div class="im"><br></div></blockquote><div>Oh, okay, that's fine.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class="im">
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
+  /// \brief Return integer type with specified width.<br>
+  virtual IntType getIntTypeByWidth(unsigned BitWidth, bool IsSigned) const;<br>
+<br>
+  /// \brief Return floating point type with specified width.<br>
+  virtual RealType getRealTypeByWidth(unsigned BitWidth) const;<br>
<br>
Instead of making these virtual, would it be possible to make these<br>
figure out the appropriate types based on the widths etc. we already<br>
compute?  e.g. for the integer types, something like so:<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>
I'd prefer to make things as simple as possible for people adding new targets.<br>
</blockquote></div>
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.<div class="im">
<br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Sorry it took a little while to get to reviewing this.<br>
</blockquote></div>
Sorry me to. I was on vocation :-)<br><span class=""><font color="#888888"><br></font></span></blockquote><div><br></div><div>-    LongDouble</div><div>+    LongDouble,</div><div>   };</div><div><br></div><div>I don't recall whether all the compilers we care about accept this without complaining; please leave out the extra comma.</div>
<div><br></div><div>Otherwise, LGTM.</div><div><br></div><div>-Eli</div></div></div></div>