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