From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa3.mentor.iphmx.com (esa3.mentor.iphmx.com [68.232.137.180]) by sourceware.org (Postfix) with ESMTPS id 711413858D28 for ; Tue, 5 Sep 2023 22:41:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 711413858D28 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com X-IronPort-AV: E=Sophos;i="6.02,230,1688457600"; d="scan'208";a="16189247" Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa3.mentor.iphmx.com with ESMTP; 05 Sep 2023 14:41:51 -0800 IronPort-SDR: TD0wqd5laMh5D6uEjRCvkEc8f9rhF4On/J9MJAJ2f36DcJE+IbzL3LmovMoP/cfxxSlDZrOn2c oadSIKX/WibTvK4grFN/JkmK6b+4fzaoqy0G+3H0juqpivS8wwE514vRek1pIDClfd5JEabVaM cyqdIMsxzQaZcXVdmpkTjJ7E9+/2FXZ8baoFY9kef1ZnYXzMMUrVyCo+JNtrg9n5ayaYNamSvz XoZnZYagw3nyiWhF3YvoH/9nb+Kho6GEb4PlyUcti30ZfA+CcM30g5VB3/iA/PwEE1gfJ5zYxV BRQ= Date: Tue, 5 Sep 2023 22:40:26 +0000 From: Joseph Myers To: Jakub Jelinek CC: Subject: Re: [PATCH 10/12] C _BitInt support [PR102989] In-Reply-To: Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-15.mgc.mentorg.com (139.181.222.15) To svr-ies-mbx-10.mgc.mentorg.com (139.181.222.10) X-Spam-Status: No, score=-3104.6 required=5.0 tests=BAYES_00,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Wed, 9 Aug 2023, Jakub Jelinek via Gcc-patches wrote: > Hi! > > This patch adds the C FE support, c-family support, small libcpp change > so that 123wb and 42uwb suffixes are handled plus glimits.h change > to define BITINT_MAXWIDTH macro. > > The previous patches really do nothing without this, which enables > all the support. Additional tests I think should be added (for things I expect should already work): * Tests for BITINT_MAXWIDTH in . Test that it's defined for C2x, but not defined for C11/C17 (the latter independent of whether the target has _BitInt support). Test the value as well: _BitInt (BITINT_MAXWIDTH) should be OK (both signed and unsigned) but _BitInt (BITINT_MAXWIDTH + 1) should not be OK. Also test that BITINT_MAXWIDTH >= ULLONG_MAX. * Test _BitInt (N) where N is a constexpr variable or enum constant (I expect these should work - the required call to convert_lvalue_to_rvalue for constexpr to work is present - but I don't see such tests in the testsuite). * Test that -funsigned-bitfields does not affect the signedness of _BitInt (N) bit-fields (the standard wording isn't entirely clear, but that's what's implemented in the patches). * Test the errors for _Sat used with _BitInt (though such a test might not actually run at present because no target supports both features). I looked at places in c-family/ and c/ that refer to INTEGER_TYPE to consider whether they should handle BITINT_TYPE and whether that matches what the patch does there. I think the following places that don't handle it probably should (with appropriate testcases added for the relevant functionality, e.g. warnings) unless there is some specific reason in each case why that's unnecessary or incorrect. c-pretty-print.cc: c_pretty_printer::direct_declarator and c_pretty_printer::declarator. c-warn.cc throughout. Maybe c-ada-spec.cc, though it might be best to ask the Ada maintainers there. c-common.cc: unsafe_conversion_p, c_common_truthvalue_conversion warnings, c_common_get_alias_set, check_builtin_function_arguments BUILT_IN_ASSUME_ALIGNED case. c-aux-info.cc (might need other support for generating appropriate names in output). c-parser.cc:c_parser_omp_clause_schedule. c-fold.cc throughout. c-typeck.c: the build_conditional_expr case where one operand is EXCESS_PRECISION_EXPR; build_c_cast; convert_for_assignment checks for integer/pointer conversions. In the parser, the syntax comment on c_parser_declspecs should have _BitInt syntax added. > + error_at (loc, "%<_BitInt%> argument %qE is not " > + "positive integer constant expression", "is not positive" should be "is not a positive" here. > --- gcc/c/c-typeck.cc.jj 2023-08-08 15:54:33.822622158 +0200 > +++ gcc/c/c-typeck.cc 2023-08-08 16:15:41.008877766 +0200 > @@ -413,10 +413,14 @@ composite_type (tree t1, tree t2) > the composite type. */ > > if (code1 == ENUMERAL_TYPE > - && (code2 == INTEGER_TYPE || code2 == BOOLEAN_TYPE)) > + && (code2 == INTEGER_TYPE > + || code2 == BOOLEAN_TYPE > + || code2 == BITINT_TYPE)) > return t1; > if (code2 == ENUMERAL_TYPE > - && (code1 == INTEGER_TYPE || code1 == BOOLEAN_TYPE)) > + && (code1 == INTEGER_TYPE > + || code1 == BOOLEAN_TYPE > + || code1 == BITINT_TYPE)) > return t2; I think these changes to handle BITINT_TYPE are unnecessary (don't do anything), since enumerated types can't have bit-precise underlying type (this code is about making a predictable choice of the enumerated type as the composite of an enumerated type and its underlying integer type). OK with those fixes. -- Joseph S. Myers joseph@codesourcery.com