PR16752[PATCH]: 'mode' attribute for unusual targets doesn't work properly.
Stepan Dyatkovskiy
stpworld at narod.ru
Tue Sep 10 04:26:35 PDT 2013
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
>
>
>
More information about the cfe-commits
mailing list