On Wed, Nov 21, 2012 at 6:26 PM, Richard Smith <[email protected]> wrote: > 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?
Yes, and nobody has bothered to fix it because the relevant functions aren't available on 32-bit platforms anyway. -Eli _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
