From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id E566638207CE for ; Fri, 16 Sep 2022 17:34:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E566638207CE Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1663349652; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references; bh=z0xM9gDiXeiwhbEWWTOkf2v/wBvP0wnCPzy38JaJ6Zw=; b=A+RFANta6pv3GOo4/Llxwrc/xGN05fh38flPOTaMty+7qFvXASlnggB46HXLxMfA6kE7Ab P98v/NqWN02tHASIsLj/XiEuoZSXfHfhkgy9YpxcvCjceDKyOLbmOSzodV8qEbYeQfKatI 3J5cY1VygHlkzCFxmTiqspD0DPlvW+M= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-205-6WYUoFllNfODFHKwfCh3Ow-1; Fri, 16 Sep 2022 13:34:09 -0400 X-MC-Unique: 6WYUoFllNfODFHKwfCh3Ow-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 0C51480206D; Fri, 16 Sep 2022 17:34:09 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.194]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A48312086F60; Fri, 16 Sep 2022 17:34:08 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 28GHY5Zw1983471 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Fri, 16 Sep 2022 19:34:05 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 28GHY4dR1983470; Fri, 16 Sep 2022 19:34:04 +0200 Date: Fri, 16 Sep 2022 19:34:03 +0200 From: Jakub Jelinek To: Jason Merrill Cc: Jonathan Wakely , "Joseph S. Myers" , Bruce Korb , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] c++: Implement P1467R9 - Extended floating-point types and standard names compiler part except for bfloat16 [PR106652] Message-ID: Reply-To: Jakub Jelinek References: MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 3.1 on 10.11.54.4 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-4.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham 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 Fri, Sep 16, 2022 at 01:48:54PM +0200, Jason Merrill wrote: > On 9/12/22 04:05, Jakub Jelinek wrote: > > The following patch implements the compiler part of C++23 > > P1467R9 - Extended floating-point types and standard names compiler part > > by introducing _Float{16,32,64,128} as keywords and builtin types > > like they are implemented for C already since GCC 7. > > It doesn't introduce _Float{32,64,128}x for C++, those remain C only > > for now, mainly because https://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangling > > has mangling for: > > ::= DF _ # ISO/IEC TS 18661 binary floating point type _FloatN (N bits) > > but doesn't for _FloatNx. And it doesn't add anything for bfloat16_t > > support, see below. > > Regarding mangling, I think mangling _FloatNx as DF x _ would be > > possible, but would need to be discussed and voted in. > > As you've seen, I opened a pull request for these. I think we can go ahead > and implement that and make sure it's resolved before the GCC 13 release. > > Or we could temporarily mangle them as an extension, i.e. u9_Float32x. > > I would expect _Float64x, at least, to be fairly popular. If we get the mangling for _Floatx agreed on, whether it is DFx_ as you proposed, or DFx or DFx_, sure, I agree we should just enable those too and will tweak the patch. It would then fix also PR85518. Though, when we add support for _Floatx and declare they are extended floating-point types, the question is about subrank comparisons, shall those have lower conversion subrank than _Float type with the same conversion rank or the other way around? Say on x86_64 where _Float32x and _Float64 has the same rank, shall (_Float32x) x + 0.0f64 have _Float32x type or _Float64? And, shall we support f32x etc. constant literal suffixes (with pedwarn always even in C++23)? > > The patch wants to keep backwards compatibility with how __float128 has > > been handled in C++ before, both for mangling and behavior in binary > > operations, overload resolution etc. So, there are some backend changes > > where for C __float128 and _Float128 are the same type (float128_type_node > > and float128t_type_node are the same pointer), but for C++ they are distinct > > types which mangle differently and _Float128 is treated as extended > > floating-point type while __float128 is treated as non-standard floating > > point type. > > How important do you think this backwards compatibility is? > > As I mentioned in the ABI proposal, I think it makes sense to make > __float128 an alias for std::float128_t, and continue using the current > mangling for __float128. I thought it is fairly important because __float128 has been around in GCC for 19 years already. To be precise, I think e.g. for x86_64 GCC 3.4 introduced it, but mangling was implemented only in GCC 4.1 (2006), before we ICEd on those. Until glibc 2.26 (2017) one had to use libquadmath when math library functions were needed, but since then one can just use libm. __float128 is on some targets (e.g. PA) just another name for long double, not a distinct type. Another thing are the PowerPC __ieee128 and __ibm128 type, I think for the former we can't make it the same type as _Float128, because e.g. libstdc++ code relies on __ieee128 and __ibm128 being long double type of the other ABI, so they should mangle as long double of the other ABI. But in that case they can't act as distinct types when long double should mangle the same as they do. And it would be weird if those types in one -mabi=*longdouble mode worked as standard floating-point type and in another as extended floating-point type, rather than just types which are neither standard nor extended as before. > I don't think we want the two types to have different semantics. If we want > to support existing __float128 code that relies on implicit narrowing > conversions, we could allow them generally with a pedwarn using the 'bad' > conversion machinery. That's probably useful anyway for better diagnostics. So you mean instead of + if (fcode == REAL_TYPE + && tcode == REAL_TYPE + && (extended_float_type_p (from) + || extended_float_type_p (to)) + && cp_compare_floating_point_conversion_ranks (from, to) >= 2) + return NULL; before conv = build_conv (ck_std, to, conv); do those checks in else if after: if ((same_type_p (to, type_promotes_to (from)) || (underlying_type && same_type_p (to, underlying_type))) && next_conversion (conv)->rank <= cr_promotion) conv->rank = cr_promotion; and pedwarn there (or somewhere later?) and set conv->bad_p = true;? I can certainly try that what will it do on the tests in the patch. > > The patch predefines __STDCPP_FLOAT{16,32,64,128}_T__ macros when > > those types are available, but only for C++23, while the underlying types > > are available in C++98 and later including the {f,F}{16,32,64,128} literal > > suffixes (but those with a pedwarn for C++20 and earlier). My understanding > > is that it needs to be predefined by the compiler, on the other side > > predefining even for older modes when is a new C++23 header > > would be weird. One can find out if _Float{16,32,64,128} is supported in > > C++ by > > defined(__FLT{16,32,64,128}_MANT_DIG__) && !defined(__FLT32X_MANT_DIG__) > > (unfortunately not just the former because GCC 7-12 predefined those too) > > or perhaps __GNUC__ >= 13 && defined(__FLT{16,32,64,128}_MANT_DIG__) > > (but that doesn't work well with older G++ 13 snapshots). > > As Joseph says, I wouldn't worry about older GCC 13 snapshots. Ok. > > As for std::bfloat16_t, three targets (aarch64, arm and x86) apparently > > "support" __bf16 type which has the bfloat16 format, but isn't really > > usable, e.g. {aarch64,arm,ix86}_invalid_conversion disallow any conversions > > from or to type with BFmode, {aarch64,arm,ix86}_invalid_unary_op disallows > > any unary operations on those except for ADDR_EXPR and > > {aarch64,arm,ix86}_invalid_binary_op disallows any binary operation on > > those. So, I think we satisfy: > > "If the implementation supports an extended floating-point type with the > > properties, as specified by ISO/IEC/IEEE 60559, of radix (b) of 2, storage > > width in bits (k) of 16, precision in bits (p) of 8, maximum exponent (emax) > > of 127, and exponent field width in bits (w) of 8, then the typedef-name > > std::bfloat16_t is defined in the header and names such a type, > > the macro __STDCPP_BFLOAT16_T__ is defined, and the floating-point literal > > suffixes bf16 and BF16 are supported." > > because we don't really support those right now. > > The question is (mainly for aarch64, arm and x86 backend maintainers) if we > > shouldn't support it, in the PR there is a partial patch to do so, but > > the big question is if it should be supported as the __bf16 type those > > 3 targets use with u6__bf16 mangling and remove those *_invalid_* cases > > and add conversions to/from at least SFmode but probably also DFmode, TFmode > > and XFmode on x86 and implement arithmetics on those through conversion to > > SFmode, performing arithmetics there and conversion back. > > Sounds good. And I've proposed DFb16_ mangling. I saw, thanks for doing that. > I'll leave that question to people more involved with FP codegen. Yeah, we'll need to discuss this with the ARM and Intel folks. In any case, that part is for incremental changes. > > Or there is the possibility to keep __bf16 a lame type one can't convert > > to/from or perform most unary and all binary ops on it, and add for C++ > > a new type (say __bfloat16_t or whatever else), agree on mangling in > > Itanium ABI and implement full support just for that type. > > Preserving the existing useless semantics doesn't sound worthwhile. Ok. Jakub