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

[Richard Smith]

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?

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/f394a2ce/attachment.html>