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

Eli Friedman eli.friedman at gmail.com
Tue Sep 17 15:09:23 PDT 2013


Sorry about the delayed response.

XC mode is not valid on PPC; if there's a test trying to use it, the test
is wrong.  Please just fix the test.

-Eli


On Tue, Sep 17, 2013 at 11:54 AM, Stepan Dyatkovskiy <stpworld at narod.ru>wrote:

> Hi Eli,
> I have to resend you PR16752 fix. Since I had changed it. The previous one
> causes failures on some buildbots.
> This fix also contains customization for PPC case:
> For 'XC' mode (96 bit float) PPC uses 128 bit fp type.
>
> -Stepan.
>
> Eli Friedman wrote:
>
>> LGTM.
>>
>> -Eli
>>
>>
>> On Mon, Sep 9, 2013 at 5:04 AM, Stepan Dyatkovskiy <stpworld at narod.ru
>> <mailto:stpworld at narod.ru>> wrote:
>>
>>     Since I had split the patch and committed only ASTContext and
>>     TargetInfo getIntTypeByWidth and friends, there is the second patch,
>>     that fixes PR16752 itself.
>>
>>     Please, find it in attachment for review.
>>
>>     -Stepan.
>>     Eli Friedman wrote:
>>
>>         On Tue, Sep 3, 2013 at 2:16 AM, Stepan Dyatkovskiy
>>         <stpworld at narod.ru <mailto:stpworld at narod.ru>
>>         <mailto:stpworld at narod.ru <mailto:stpworld at narod.ru>>> wrote:
>>
>>              Hi Eli,
>>              Sorry for latency.
>>              As you remember this patch should correct 'mode' attr
>>              implementation. You proposed to use generic way of type
>>         detection
>>              for each target: just scan for target types and select one
>> with
>>              suitable width.
>>
>>              Unfortunately I can't commit patch with generic
>>         implementation. I
>>              got test failure for clang. And I expect more failures for
>>         another
>>              targets. The reason is next.
>>
>>              The target could still keep mode unsupported even if it has
>>         type of
>>              suitable width. I mean the next.
>>              For example this string should cause error for i686
>>         machines (128
>>              bit float):
>>              typedef          float f128ibm __attribute__ ((mode (TF)));
>>              But in case of generic approach for all targets it would be
>>         passed.
>>              In this case virtual functions allows to implement
>> exceptions.
>>
>>
>>         The generic implementation is still essentially correct; the the
>>         issue
>>         is that getLongDoubleWidth() isn't actually the right value to
>>         check.
>>            It returns "sizeof(long double) * 8", not the actual width of
>> the
>>         underlying float format.  You should be able to work around this
>> by
>>         checking getLongDoubleFormat() instead of getLongDoubleWidth().
>>
>>              In this case 'getIntTypeByWidth' may be a bit confusing name.
>>              Instead it is supposed to return type for some particular
>>         mode, not
>>              by width. Something like getInt/RealTypeForMode(width, sign).
>>              Though, currently I kept the original name.
>>
>>
>>         If you want to change it, that's fine.
>>
>>         -Eli
>>
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130917/a63c50fd/attachment.html>


More information about the cfe-commits mailing list