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

[Stepan Dyatkovskiy]

 > Tried to commit the second patch, but some buildbots did like it. It
Sorry for typo. Buildbots *didn't* like it.
Tests were passed on my computer, since I disabled ppc target.
-Stepan.

Stepan Dyatkovskiy wrote:
> Hi Eli,
> Tried to commit the second patch, but some buildbots did like it. It
> only works if we add one more branch to getRealTypeByWidth:
>
> diff --git a/lib/Basic/TargetInfo.cpp b/lib/Basic/TargetInfo.cpp
> index 38cb411..bf6acd2 100644
> --- a/lib/Basic/TargetInfo.cpp
> +++ b/lib/Basic/TargetInfo.cpp
> @@ -174,7 +174,10 @@ TargetInfo::RealType
> TargetInfo::getRealTypeByWidth(unsigned BitWidth) const {
>
>     switch (BitWidth) {
>     case 96:
> -    if (&getLongDoubleFormat() == &llvm::APFloat::x87DoubleExtended)
> +    if (&getLongDoubleFormat() == &llvm::APFloat::x87DoubleExtended ||
> +        // Due to test/Sema/attr-mode.c test it seems that
> +        // PPC machines should use PPCDoubleDouble type for XC mode.
> +        &getLongDoubleFormat() == &llvm::APFloat::PPCDoubleDouble)
>         return LongDouble;
>       break;
>     case 128:
>
> If its OK, I would like to commit patch and this change.
>
> -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
>>
>>
>>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits