PR16752[PATCH]: 'mode' attribute for unusual targets doesn't work properly.
eli.friedman at gmail.com
Mon Sep 9 11:58:54 PDT 2013
On Mon, Sep 9, 2013 at 5:04 AM, Stepan Dyatkovskiy <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.
> Eli Friedman wrote:
>> On Tue, Sep 3, 2013 at 2:16 AM, Stepan Dyatkovskiy <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.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the cfe-commits