[cfe-commits] [PATCH] Improved handling of 128-bit integer literals

[Richard Smith]

On Thu, Nov 29, 2012 at 4:16 PM, Richard Smith <richard at metafoo.co.uk>wrote:

> Generally, we still need to handle Eli's observation that the C and C++
> extended integer type rules require us to make intmax_t be __int128 if
> we're going to allow literals to be of type __int128. I'm inclined to say
> we should bite the bullet here, and treat __int128 as a proper extended
> integer type (and thus change intmax_t, preprocessor constant expressions,
> and so on). Any objections?
>

OK, we can't do that without breaking printf's %jd format specifier. Let's
leave intmax_t as is, for now at least.


> You can go ahead with this patch and (you or someone else) make the
> intmax_t changes afterwards, if we have consensus on that direction.
>
> Index: lib/Sema/SemaDeclAttr.cpp
> ===================================================================
> --- lib/Sema/SemaDeclAttr.cpp (revision 168447)
> +++ lib/Sema/SemaDeclAttr.cpp (working copy)
> @@ -1114,6 +1114,12 @@
>        return;
>      }
>
> +    if (!ArgNum.isIntN(64)) {
> +      S.Diag(Attr.getLoc(), diag::err_attribute_argument_out_of_bounds)
> +      << "alloc_size" << I.getArgNum() << Ex->getSourceRange();
>
> This line needs more indentation.
>
> Index: lib/Sema/SemaExpr.cpp
> ===================================================================
> --- lib/Sema/SemaExpr.cpp (revision 168447)
> +++ lib/Sema/SemaExpr.cpp (working copy)
> [...]
> +      // Microsoft literals simply *are* the specified type, regardless of
> +      // whether the value fits into it or not, or whether they are hex or
> +      // decimal or octal.
> +      if (Literal.isLongLong) {
> +        Width = Context.getTargetInfo().getLongLongWidth();
> +        Ty = Literal.isUnsigned ? Context.UnsignedLongLongTy :
> +          Context.LongLongTy;
> +      } else if (Literal.isLong) {
> +        Width = Context.getTargetInfo().getLongWidth();
> +        Ty = Literal.isUnsigned ? Context.UnsignedLongTy : Context.LongTy;
> +      } else {
> +        Width = Context.getTargetInfo().getIntWidth();
> +        Ty = Literal.isUnsigned ? Context.UnsignedIntTy : Context.IntTy;
>        }
>
> Hmm, it looks incorrect to use isLong here instead of checking
> microsoftWidth. In particular, if I use -fms-extensions on Linux, this
> static_assert fails:
>
> static_assert(sizeof(0i32) == 4, "");
>
> ... because 'long' is a 64-bit type on my target. (We also incorrectly use
> 'int' for i16 and i8.) We should presumably use the smallest integer type
> which is at least N bits wide. I'm OK with this being dealt with in a
> separate patch, since it's not a regression.
>
> +      if (ResultVal.getActiveBits() > (Literal.isUnsigned ?
> +                                       Literal.microsoftWidth :
> +                                       Literal.microsoftWidth - 1)) {
> +        Diag(Tok.getLocation(), diag::warn_integer_too_large);
> +        // APInt asserts on NOP-truncations, so we need to be a little bit
> +        // carefull; otherwise we would crash when the literal is signed
> and
> +        // microsoftWidth == Width.
> +        if (Literal.microsoftWidth < Width)
> +          ResultVal = ResultVal.trunc(Literal.microsoftWidth);
>
> This can be simplified somewhat by passing Literal.microsoftWidth instead
> of 64 when constructing ResultVal (you'd still need to check for the high
> bit being set for a signed literal).
>
>        }
> +
> +      // Unlike in the non-MS-literal case, we may have already truncated
> the
> +      // APInt to be smaller than the final literal, so we may need to
> adjust
> +      // the size in either direciton here; use sextOrTrunc instead of
> trunc.
> +      if (ResultVal.getBitWidth() != Width)
> +        ResultVal = ResultVal.sextOrTrunc(Width);
>
>  You should zext if the literal is unsigned (maybe make ResultVal an
> APSInt, and use extOrTrunc). For instance, 0x18000Ui16 will currently first
> be truncated to 0x8000 (with a warning), then sign extended to 0x80000000,
> rather than producing the desired value 0x00008000.
>
> +      // Checking for 64-bit pointers is perhaps not the correct way to
> detect
> +      // i128 support, but it seems to be what is done elsewhere.
> +      if (PP.getTargetInfo().getPointerWidth(0) >= 64) {
>
> TargetInfo now has a hasInt128Type().
>
> On Sat, Nov 24, 2012 at 8:59 AM, Stephen Canon <scanon at apple.com> wrote:
>
>> Updated patch with feedback from the first round.  Please review.
>>
>> Thanks!
>> - Steve
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121129/5ca6576c/attachment.html>