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 EB62B3858D37 for ; Thu, 20 Apr 2023 12:18:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org EB62B3858D37 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=1681993129; 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=184iYRKUKmaPIiT/6ruerU+1eKUim6UPrFh8GF7Zj/Q=; b=RPgG6Ew2yclu0wFsF+rEygCzEe3RjmybZQRF5UK267uVnlPo56Q/5RFrJj+bRzqmmF0/29 xeHT7HuDVnSmlj6ffMlF5ayGCgq6CoAAqRqhn5fz5rSzSVkQZk1i9/5mnOKfnY+YaUMfXH 9oA7mpYd5NMLOJnlP7l1UkaxRYV9Fwk= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-583-MQXF2dUFP2KH4aDNm2PAPw-1; Thu, 20 Apr 2023 08:18:46 -0400 X-MC-Unique: MQXF2dUFP2KH4aDNm2PAPw-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 550CA29ABA07; Thu, 20 Apr 2023 12:18:46 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.194.25]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 16EF52026D16; Thu, 20 Apr 2023 12:18:45 +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 33KCIhwL1981037 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Thu, 20 Apr 2023 14:18:43 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 33KCIgr71981036; Thu, 20 Apr 2023 14:18:42 +0200 Date: Thu, 20 Apr 2023 14:18:42 +0200 From: Jakub Jelinek To: liuhongt Cc: gcc-patches@gcc.gnu.org, crazylht@gmail.com, hjl.tools@gmail.com, ubizjak@gmail.com Subject: Re: [PATCH] [i386] Support type _Float16/__bf16 independent of SSE2. Message-ID: Reply-To: Jakub Jelinek References: <20230419071551.3478647-1-hongtao.liu@intel.com> MIME-Version: 1.0 In-Reply-To: <20230419071551.3478647-1-hongtao.liu@intel.com> 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=-3.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE 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 Wed, Apr 19, 2023 at 03:15:51PM +0800, liuhongt wrote: ChangeLog nits have been already reported earlier. > --- a/gcc/config/i386/i386-c.cc > +++ b/gcc/config/i386/i386-c.cc > @@ -817,6 +817,43 @@ ix86_target_macros (void) > if (!TARGET_80387) > cpp_define (parse_in, "_SOFT_FLOAT"); > > + /* HFmode/BFmode is supported without depending any isa > + in scalar_mode_supported_p and libgcc_floating_mode_supported_p, > + but according to psABI, they're really supported w/ SSE2 and above. > + Since libstdc++ uses __STDCPP_FLOAT16_T__ and __STDCPP_BFLOAT16_T__ > + for backend support of the types, undef the macros to avoid > + build failure, see PR109504. */ > + if (!TARGET_SSE2) > + { > + if (c_dialect_cxx () > + && cxx_dialect > cxx20) Formatting, both conditions are short, so just put them on one line. > + { > + cpp_undef (parse_in, "__STDCPP_FLOAT16_T__"); > + cpp_undef (parse_in, "__STDCPP_BFLOAT16_T__"); > + } But for the C++23 macros, more importantly I think we really should also in ix86_target_macros_internal add if (c_dialect_cxx () && cxx_dialect > cxx20 && (isa_flag & OPTION_MASK_ISA_SSE2)) { def_or_undef (parse_in, "__STDCPP_FLOAT16_T__"); def_or_undef (parse_in, "__STDCPP_BFLOAT16_T__"); } plus associated libstdc++ changes. It can be done incrementally though. > + > + if (flag_building_libgcc) > + { > + /* libbid uses __LIBGCC_HAS_HF_MODE__ and __LIBGCC_HAS_BF_MODE__ > + to check backend support of _Float16 and __bf16 type. */ That is actually the case only for HFmode, but not for BFmode right now. So, we need further work. One is to add the BFmode support in there, and another one is make sure the _Float16 <-> _Decimal* and __bf16 <-> _Decimal* conversions are compiled in also if not -msse2 by default. One way to do that is wrap the HF and BF mode related functions on x86 #ifndef __SSE2__ into the pragmas like intrin headers use (but then perhaps we don't need to undef this stuff here), another is not provide the hf/bf support in that case from the TUs where they are provided now, but from a different one which would be compiled with -msse2. > + cpp_undef (parse_in, "__LIBGCC_HAS_HF_MODE__"); > + cpp_undef (parse_in, "__LIBGCC_HF_FUNC_EXT__"); > + cpp_undef (parse_in, "__LIBGCC_HF_MANT_DIG__"); > + cpp_undef (parse_in, "__LIBGCC_HF_EXCESS_PRECISION__"); > + cpp_undef (parse_in, "__LIBGCC_HF_EPSILON__"); > + cpp_undef (parse_in, "__LIBGCC_HF_MAX__"); > + cpp_undef (parse_in, "__LIBGCC_HF_MIN__"); > + > + cpp_undef (parse_in, "__LIBGCC_HAS_BF_MODE__"); > + cpp_undef (parse_in, "__LIBGCC_BF_FUNC_EXT__"); > + cpp_undef (parse_in, "__LIBGCC_BF_MANT_DIG__"); > + cpp_undef (parse_in, "__LIBGCC_BF_EXCESS_PRECISION__"); > + cpp_undef (parse_in, "__LIBGCC_BF_EPSILON__"); > + cpp_undef (parse_in, "__LIBGCC_BF_MAX__"); > + cpp_undef (parse_in, "__LIBGCC_BF_MIN__"); > + } > + } > + > --- a/gcc/config/i386/i386.cc > +++ b/gcc/config/i386/i386.cc > @@ -2651,7 +2651,10 @@ construct_container (machine_mode mode, machine_mode orig_mode, > > /* We allowed the user to turn off SSE for kernel mode. Don't crash if > some less clueful developer tries to use floating-point anyway. */ > - if (needed_sseregs && !TARGET_SSE) > + if (needed_sseregs > + && (!TARGET_SSE > + || (VALID_SSE2_TYPE_MODE (mode) > + && !TARGET_SSE2))) Formatting, no need to split this up that much. if (needed_sseregs && (!TARGET_SSE || (VALID_SSE2_TYPE_MODE (mode) && !TARGET_SSE2))) or even better if (needed_sseregs && (!TARGET_SSE || (VALID_SSE2_TYPE_MODE (mode) && !TARGET_SSE2))) will do it. > @@ -22805,9 +22827,10 @@ ix86_emit_support_tinfos (emit_support_tinfos_callback callback) > > if (!TARGET_SSE2) > { > - gcc_checking_assert (!float16_type_node && !bfloat16_type_node); > - float16_type_node = ix86_float16_type_node; > - bfloat16_type_node = ix86_bf16_type_node; > + float16_type_node > + = float16_type_node ? float16_type_node : ix86_float16_type_node; > + bfloat16_type_node > + = bfloat16_type_node ? bfloat16_type_node : ix86_bf16_type_node; > callback (float16_type_node); > callback (bfloat16_type_node); Instead of this, just use if (!float16_type_node) { float16_type_node = ix86_float16_type_node; callback (float16_type_node); float16_type_node = NULL_TREE; } if (!bfloat16_type_node) { bfloat16_type_node = ix86_bf16_type_node; callback (bfloat16_type_node); bfloat16_type_node = NULL_TREE; } ? > +/* Return the diagnostic message string if conversion from FROMTYPE to > + TOTYPE is not allowed, NULL otherwise. */ > + > +static const char * > +ix86_invalid_conversion (const_tree fromtype, const_tree totype) > +{ > + if (element_mode (fromtype) != element_mode (totype)) > + { > + /* Do no allow conversions to/from BFmode/HFmode scalar types > + when TARGET_SSE2 is not available. */ > + if ((TYPE_MODE (fromtype) == BFmode > + || TYPE_MODE (fromtype) == HFmode) > + && !TARGET_SSE2) First of all, not really sure if this should be purely about scalar modes, not also complex and vector modes involving those inner modes. Because complex or vector modes with BF/HF elements will be without TARGET_SSE2 for sure lowered into scalar code and that can't be handled either. So if (!TARGET_SSE2 && GET_MODE_INNER (TYPE_MODE (fromtype)) == BFmode) or even better if (!TARGET_SSE2 && element_mode (fromtype) == BFmode) ? Or even better remember the 2 modes above into machine_mode temporaries and just use those in the != comparison and for the checks? Also, I think it is weird to tell user %<__bf16%> or %<_Float16%> when we know which one it is. Just return separate messages? > + return N_("invalid conversion from type %<__bf16%> " > + "or %<_Float16%> without option %<-msse2%>"); > + > + if ((TYPE_MODE (totype) == BFmode > + || TYPE_MODE (totype) == HFmode) > + && !TARGET_SSE2) > + return N_("invalid conversion to type %<__bf16%> " > + "or %<_Float16%> without option %<-msse2%>"); Ditto. > + } > + > + /* Conversion allowed. */ > + return NULL; > +} > + > +/* Return the diagnostic message string if the unary operation OP is > + not permitted on TYPE, NULL otherwise. */ > + > +static const char * > +ix86_invalid_unary_op (int op, const_tree type) > +{ > + /* Reject all single-operand operations on BFmode/HFmode except for & > + when TARGET_SSE2 is not available. */ > + if ((element_mode (type) == BFmode || element_mode (type) == HFmode) > + && !TARGET_SSE2 && op != ADDR_EXPR) > + return N_("operation not permitted on type %<__bf16%> " > + "or %<_Float16%> without option %<-msse2%>"); Similarly. Also, check !TARGET_SSE2 first as inexpensive one. > + > + /* Operation allowed. */ > + return NULL; > +} > + > +/* Return the diagnostic message string if the binary operation OP is > + not permitted on TYPE1 and TYPE2, NULL otherwise. */ > + > +static const char * > +ix86_invalid_binary_op (int op ATTRIBUTE_UNUSED, const_tree type1, > + const_tree type2) > +{ > + /* Reject all 2-operand operations on BFmode or HFmode > + when TARGET_SSE2 is not available. */ > + if ((element_mode (type1) == BFmode > + || element_mode (type2) == BFmode > + || element_mode (type1) == HFmode > + || element_mode (type2) == HFmode) > + && !TARGET_SSE2) > + return N_("operation not permitted on type %<__bf16%> " > + "or %<_Float16%> without option %<-msse2%>"); Similarly. Jakub