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 CC6C33858D35 for ; Sat, 5 Feb 2022 00:01:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CC6C33858D35 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-295-CnRne-PhOLen9ymKxFy1Wg-1; Fri, 04 Feb 2022 19:01:37 -0500 X-MC-Unique: CnRne-PhOLen9ymKxFy1Wg-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 5C99264140; Sat, 5 Feb 2022 00:01:36 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.125]) by smtp.corp.redhat.com (Postfix) with ESMTPS id E7AFD5DBBB; Sat, 5 Feb 2022 00:01:35 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.16.1/8.16.1) with ESMTPS id 21501XTN2029286 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Sat, 5 Feb 2022 01:01:33 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.16.1/8.16.1/Submit) id 214N2Ei32027464; Sat, 5 Feb 2022 00:02:14 +0100 Date: Sat, 5 Feb 2022 00:02:14 +0100 From: Jakub Jelinek To: Jason Merrill Cc: Richard Biener , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] c++, v3: Further address_compare fixes [PR89074] Message-ID: <20220204230030.GO2646553@tucnak> Reply-To: Jakub Jelinek References: <841aeb56-57a3-07b3-dbc0-a45ef0cca554@redhat.com> <20220118164048.GD2646553@tucnak> <20220203155237.GR2646553@tucnak> <191d40b7-9de1-879b-2a39-8b26c869c955@redhat.com> <20220203203320.GV2646553@tucnak> <14f595e5-6158-6305-ef50-ac8d5f2b3c87@redhat.com> <20220203211838.GW2646553@tucnak> <0a674670-8ae0-5219-c96a-8f5219e6605a@redhat.com> <20220204134145.GE2646553@tucnak> <8b2d8d85-c723-f82d-1d0e-9ef72e0cef07@redhat.com> MIME-Version: 1.0 In-Reply-To: <8b2d8d85-c723-f82d-1d0e-9ef72e0cef07@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 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=-5.2 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.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 05 Feb 2022 00:01:42 -0000 On Fri, Feb 04, 2022 at 04:42:41PM -0500, Jason Merrill wrote: > > @@ -20,9 +20,16 @@ along with GCC; see the file COPYING3. > > #ifndef GCC_FOLD_CONST_H > > #define GCC_FOLD_CONST_H > > -/* Non-zero if we are folding constants inside an initializer; zero > > - otherwise. */ > > +/* Nonzero if we are folding constants inside an initializer or a C++ > > + manifestly-constant-evaluated context; zero otherwise. > > + Should be used when folding in initializer enables additional > > + optimizations. */ > > extern int folding_initializer; > > +/* Nonzer of we are folding C++ manifestly-constant-evaluated context; zero > > Still need to fix this typo. Sorry, finally now fixed in my copy. > > + otherwise. > > + Should be used when certain constructs shouldn't be optimized > > + during folding in that context. */ > > +bool folding_cxx_constexpr = false; > > + > > /* The following constants represent a bit based encoding of GCC's > > comparison operators. This encoding simplifies transformations > > on relational comparison operators, such as AND and OR. */ > > @@ -16628,41 +16636,55 @@ address_compare (tree_code code, tree ty > > Incidentally, the function comment needs to document TYPE. Will do. > So at this point equal can be 0 or 2, the latter because either the offset > is out of bounds, or because we're comparing offset 0 to one-past-the-end. Yes. > In the out-of-bounds case, we're into undefined behavior, so I'd be inclined > to return 2 immediately rather than continue, so the code below only needs > to worry about possibly overlapping/contiguous objects. You mean for folding_cxx_constexpr ? The code does that basically, with one exception, the folding_initializer FUNCTION_DECL cmp FUNCTION_DECL case. We don't track sizes of functions, so the size of 1 is just a hack to pretend functions don't have zero size. Some functions can have zero size if they contain just __builtin_unreachable, but it is very rare. But I guess I could move that if (folding_initializer && TREE_CODE (base0) == FUNCTION_DECL && TREE_CODE (base1) == FUNCTION_DECL) return 0; above the size checking block and then indeed right after that do if (folding_cxx_constexpr && equal) return equal; with a comment. > In the code below, in !constexpr mode we decide to return 0 even though > equal == 2 in three cases which need more commentary, either together or > separately: > > 1) One is a string and the other a decl. Do we know that we can't layout a > string and a global variable next to each other? This overlaps a lot > with... > > 2) We're comparing as pointers (rather than integers), so we return unequal > even if they could be equal in practice if the objects are contiguous. The > comment says this but still needs a rationale; it doesn't seem useful to me > for the limited cases that could reach here with equal == 2. For the pointer comparisons we just exploit the undefined behavior and pretend they can't be adjacent even if they actually sometimes can be, and we've been doing that intentionally for years. If one does (uintptr_t) &x == (uintptr_t) &y, we try to be more conservative. > 3) We're comparing a local variable and a global, so they really can't be > equal unless the offset is far out of bounds. This is currently last; we > might move it first and treat strings like globals? For the automatic vs. global or strings, it is very unlikely they'd be adjacent, with typical memory layouts there couldn't be any heap and stack would need to grow until it reaches end of data or bss section on a page boundary. Strings perhaps could be adjacent in .rodata, but again it is fairly rare. And sure, I can try to improve comments. Jakub