On Thu, Nov 29, 2012 at 4:16 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>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?</div>
</blockquote><div><br></div><div>OK, we can't do that without breaking printf's %jd format specifier. Let's leave intmax_t as is, for now at least.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>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.</div><div><br></div><div>Index: lib/Sema/SemaDeclAttr.cpp</div><div>
===================================================================</div><div>--- lib/Sema/SemaDeclAttr.cpp<span style="white-space:pre-wrap"> </span>(revision 168447)</div>
<div>+++ lib/Sema/SemaDeclAttr.cpp<span style="white-space:pre-wrap"> </span>(working copy)</div><div>@@ -1114,6 +1114,12 @@</div><div> return;</div><div class="im"><div> }</div><div> </div><div>+ if (!ArgNum.isIntN(64)) {</div>
<div>+ S.Diag(Attr.getLoc(), diag::err_attribute_argument_out_of_bounds)</div><div>+ << "alloc_size" << I.getArgNum() << Ex->getSourceRange();</div><div><br></div></div><div>This line needs more indentation.</div>
<div><br></div><div><div>Index: lib/Sema/SemaExpr.cpp</div><div>===================================================================</div><div>--- lib/Sema/SemaExpr.cpp<span style="white-space:pre-wrap"> </span>(revision 168447)</div>
<div>+++ lib/Sema/SemaExpr.cpp<span style="white-space:pre-wrap"> </span>(working copy)</div></div><div>[...]</div><div>+ // Microsoft literals simply *are* the specified type, regardless of</div><div>
+ // whether the value fits into it or not, or whether they are hex or</div><div>+ // decimal or octal.</div>
<div>+ if (Literal.isLongLong) {</div><div>+ Width = Context.getTargetInfo().getLongLongWidth();</div><div>+ Ty = Literal.isUnsigned ? Context.UnsignedLongLongTy :</div><div>+ Context.LongLongTy;</div>
<div>+ } else if (Literal.isLong) {</div><div>+ Width = Context.getTargetInfo().getLongWidth();</div><div>+ Ty = Literal.isUnsigned ? Context.UnsignedLongTy : Context.LongTy;</div><div>+ } else {</div>
<div>+ Width = Context.getTargetInfo().getIntWidth();</div><div>+ Ty = Literal.isUnsigned ? Context.UnsignedIntTy : Context.IntTy;</div><div> }</div><div><br></div><div>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:</div>
<div><br></div><div><div>static_assert(sizeof(0i32) == 4, "");</div></div><div><br></div><div>... 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.</div>
<div><br></div><div>+ if (ResultVal.getActiveBits() > (Literal.isUnsigned ?</div><div><div><div>+ Literal.microsoftWidth :</div><div>+ Literal.microsoftWidth - 1)) {</div>
<div>+ Diag(Tok.getLocation(), diag::warn_integer_too_large);</div><div>+ // APInt asserts on NOP-truncations, so we need to be a little bit</div>
<div>+ // carefull; otherwise we would crash when the literal is signed and</div><div>+ // microsoftWidth == Width.</div><div>+ if (Literal.microsoftWidth < Width)</div><div>+ ResultVal = ResultVal.trunc(Literal.microsoftWidth);</div>
<div><div><br>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).</div>
<div><br></div></div>
<div> }</div></div><div>+ </div></div><div>+ // Unlike in the non-MS-literal case, we may have already truncated the</div><div><div>+ // APInt to be smaller than the final literal, so we may need to adjust</div>
<div>+ // the size in either direciton here; use sextOrTrunc instead of trunc.</div><div class="im"><div>+ if (ResultVal.getBitWidth() != Width)</div></div><div>+ ResultVal = ResultVal.sextOrTrunc(Width);</div>
</div><div><br>
</div>
<div>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.</div>
<div><br></div><div><div>+ // Checking for 64-bit pointers is perhaps not the correct way to detect</div><div>+ // i128 support, but it seems to be what is done elsewhere.</div><div>+ if (PP.getTargetInfo().getPointerWidth(0) >= 64) {</div>
</div><div><br></div><div>TargetInfo now has a hasInt128Type().</div><div class="HOEnZb"><div class="h5"><div><br></div><div class="gmail_quote">On Sat, Nov 24, 2012 at 8:59 AM, Stephen Canon <span dir="ltr"><<a href="mailto:scanon@apple.com" target="_blank">scanon@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Updated patch with feedback from the first round. Please review.<br>
<br>
Thanks!<br>
- Steve<br>
<br>
</blockquote></div><br>
</div></div></blockquote></div><br>