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 4B11E3858C2F for ; Mon, 15 Aug 2022 20:21:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4B11E3858C2F 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-527-n0GHP4zZOv-BDlqBDDa7yg-1; Mon, 15 Aug 2022 16:21:47 -0400 X-MC-Unique: n0GHP4zZOv-BDlqBDDa7yg-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id A72D0180F6E3; Mon, 15 Aug 2022 20:21:46 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.41]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 6477818EA8; Mon, 15 Aug 2022 20:21:46 +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 27FKLhW01250143 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Mon, 15 Aug 2022 22:21:44 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 27FKLh2q1250142; Mon, 15 Aug 2022 22:21:43 +0200 Date: Mon, 15 Aug 2022 22:21:42 +0200 From: Jakub Jelinek To: FX Cc: gcc-patches@gcc.gnu.org, fortran@gcc.gnu.org Subject: Re: [PATCH] fortran: Expand ieee_arithmetic module's ieee_value inline [PR106579] Message-ID: Reply-To: Jakub Jelinek References: <4B7DE3A3-8F13-49D1-BCA9-723360EC386A@gmail.com> MIME-Version: 1.0 In-Reply-To: <4B7DE3A3-8F13-49D1-BCA9-723360EC386A@gmail.com> X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit 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, 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 X-BeenThere: fortran@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Fortran mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 Aug 2022 20:21:51 -0000 On Mon, Aug 15, 2022 at 10:00:02PM +0200, FX wrote: > I have two questions, on this and the ieee_class patch: > > > > + tree type = TREE_TYPE (arg); > > + gcc_assert (TREE_CODE (type) == RECORD_TYPE); > > + tree field = NULL_TREE; > > + for (tree f = TYPE_FIELDS (type); f != NULL_TREE; f = DECL_CHAIN (f)) > > + if (TREE_CODE (f) == FIELD_DECL) > > + { > > + gcc_assert (field == NULL_TREE); > > + field = f; > > + } > > + gcc_assert (field); > > Why looping over fields? The class type is a simple type with only one member (and it should be an integer, we can assert that). I wanted to make sure it has exactly one field. The ieee_arithmetic.F90 module in libgfortran surely does that, but I've been worrying about some user overriding that module with something different. At least in the C/C++ FEs we had in the past tons of bugs filed for when some builtin made some assumptions about some headers and data types in those and then somebody running a testcase that violated that. Even failed gcc_assertion isn't the best answer to that, ideally one would verify that upfront and then either error, sorry or ignore the call (leave it as is). In that last case, it might be better to do the check on the gfortran FE types instead of trees (verify the return type or second argument type is ieee_class_type derived type with a single integral (hidden) field). > > + case IEEE_POSITIVE_ZERO: > > + /* Make this also the default: label. */ > > + label = gfc_build_label_decl (NULL_TREE); > > + tmp = build_case_label (NULL_TREE, NULL_TREE, label); > > + gfc_add_expr_to_block (&body, tmp); > > + real_from_integer (&real, TYPE_MODE (type), 0, SIGNED); > > + break; > > Do we need a default label? It’s not like this is a more likely case than anything else… The libgfortran version had default: label: switch (type) \ { \ case IEEE_SIGNALING_NAN: \ return __builtin_nans ## SUFFIX (""); \ case IEEE_QUIET_NAN: \ return __builtin_nan ## SUFFIX (""); \ case IEEE_NEGATIVE_INF: \ return - __builtin_inf ## SUFFIX (); \ case IEEE_NEGATIVE_NORMAL: \ return -42; \ case IEEE_NEGATIVE_DENORMAL: \ return -(GFC_REAL_ ## TYPE ## _TINY) / 2; \ case IEEE_NEGATIVE_ZERO: \ return -(GFC_REAL_ ## TYPE) 0; \ case IEEE_POSITIVE_ZERO: \ return 0; \ case IEEE_POSITIVE_DENORMAL: \ return (GFC_REAL_ ## TYPE ## _TINY) / 2; \ case IEEE_POSITIVE_NORMAL: \ return 42; \ case IEEE_POSITIVE_INF: \ return __builtin_inf ## SUFFIX (); \ default: \ return 0; \ } \ and I've tried to traslate that into what it generates. There is at least the IEEE_OTHER_VALUE (aka 0) value that isn't covered in the switch, but it is just an integer under the hood, so it could have any other value. Jakub