<div dir="ltr">LGTM.<br><br>-Eli<br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Sep 9, 2013 at 5:04 AM, Stepan Dyatkovskiy <span dir="ltr"><<a href="mailto:stpworld@narod.ru" target="_blank">stpworld@narod.ru</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Since I had split the patch and committed only ASTContext and TargetInfo getIntTypeByWidth and friends, there is the second patch, that fixes PR16752 itself.<br>

<br>
Please, find it in attachment for review.<br>
<br>
-Stepan.<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 Tue, Sep 3, 2013 at 2:16 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>
    Hi Eli,<br>
    Sorry for latency.<br>
    As you remember this patch should correct 'mode' attr<br>
    implementation. You proposed to use generic way of type detection<br>
    for each target: just scan for target types and select one with<br>
    suitable width.<br>
<br>
    Unfortunately I can't commit patch with generic implementation. I<br>
    got test failure for clang. And I expect more failures for another<br>
    targets. The reason is next.<br>
<br>
    The target could still keep mode unsupported even if it has type of<br>
    suitable width. I mean the next.<br>
    For example this string should cause error for i686 machines (128<br>
    bit float):<br>
    typedef          float f128ibm __attribute__ ((mode (TF)));<br>
    But in case of generic approach for all targets it would be passed.<br>
    In this case virtual functions allows to implement exceptions.<br>
<br>
<br>
The generic implementation is still essentially correct; the the issue<br>
is that getLongDoubleWidth() isn't actually the right value to check.<br>
  It returns "sizeof(long double) * 8", not the actual width of the<br>
underlying float format.  You should be able to work around this by<br>
checking getLongDoubleFormat() instead of getLongDoubleWidth().<br>
<br>
    In this case 'getIntTypeByWidth' may be a bit confusing name.<br>
    Instead it is supposed to return type for some particular mode, not<br>
    by width. Something like getInt/RealTypeForMode(width, sign).<br>
    Though, currently I kept the original name.<br>
<br>
<br>
If you want to change it, that's fine.<br>
<br>
-Eli<br>
<br>
</div></div></blockquote>
<br>
</blockquote></div><br></div>