On Wed, Nov 21, 2012 at 6:15 PM, Eli Friedman <[email protected]> wrote: > On Wed, Nov 21, 2012 at 5:44 PM, Richard Smith <[email protected]> wrote: >> On Wed, Nov 21, 2012 at 3:48 PM, Stephen Canon <[email protected]> wrote: >>> >>> On Nov 21, 2012, at 5:10 PM, Dmitri Gribenko <[email protected]> wrote: >>> >>>> There are similar issues in 'nonnull', 'ownership' and 'format' >>>> attributes. I have an incomplete patch for all these, that refactors >>>> duplicated code over handle*() functions in SemaDeclAttr.cpp and fixes >>>> this 128-bit issue, but I decided not to submit it until the semantic >>>> analysis is fixed. >>> >>> Great, thanks. >>> >>>> I see that the code is much cleaner when FitsIn* are computed upfront, >>>> but this leads to some extra work -- each isIntN() boils down to >>>> counting leading zeros. Is there a clean way to defer the computation >>>> to the point where it is required? I don't know how hot this code is, >>>> so maybe this is not worth doing. >>> >>> In theory the compiler can do this optimization for us, at least in the >>> common case of "small" literals (where I believe everything is in the APInt >>> header). >> >> You could directly call ResultVal.getActiveBits() once, rather than >> repeatedly calling isIntN. It'd also be great to factor out some of >> the repetition here. >> >> For the MS suffix case, how about... >> >> if (Literal.isLongLong) { >> Width = Context.getTargetInfo().getLongLongWidth(); >> Ty = Literal.isUnsigned ? Context.UnsignedLongLongTy : Context.LongLongTy; >> } else if (Literal.isLong) { >> // ... >> } else { >> // ... >> >>>> This LGTM with tests and code style changes mentioned above, but >>>> please wait for Richard Smith's review. >>> >>> Great, I'll add the tests you requested and fix the typos in the meantime. >> >> + // If we are in MSVC mode, we pretend that "LL" is a microsoft literal >> + // suffix in order to get the expected (wrong) behavior. >> + if (getLangOpts().MicrosoftExt && Literal.isLongLong) { >> + Literal.isMicrosoftInteger = true; >> + } >> >> This should check MicrosoftMode, not MicrosoftExt, since it changes >> the behavior of conforming code. Also, no braces here. >> >> + if (ResultVal.getBitWidth() != Width) >> + ResultVal = ResultVal.trunc(Width); >> >> Have you considered producing the warn_integer_too_large diagnostic if >> we truncate here? >> >> + // We will evaluate literals in an "extended integer type" as allowed >> by >> + // the C and C++ standards. On LP64 platforms (which have >> __[u]int128_t) >> + // we use that type. However, we can't use it on other platforms, or >> + // else we would generate arithmetic using those types and crash when >> we >> + // try to codegen. If we don't have LP64, we use [unsigned] long long >> + // instead. >> >> We currently provide __int128 on all platforms. If the legalizer can't >> cope with that on some platform, then we have a problem. You're right >> that we only provide the __int128_t and __uint128_t typedefs on >> platforms with 64-bit pointers, though that restriction dates back to >> r70480, when I would expect the legalizer was significantly more >> limited. We might want to revisit that now. > > We still don't have any way to legalize 128-bit multiplication and > division on 32-bit platforms.
I see, this is enough to crash Clang: __int128 n, m = n*n; It seems we try to produce a libcall which returns 4 i32s, and the calling convention doesn't support that? _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
