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.133.124]) by sourceware.org (Postfix) with ESMTPS id 2E6EE3858417 for ; Wed, 7 Feb 2024 16:37:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2E6EE3858417 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 2E6EE3858417 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707323826; cv=none; b=WS4mKd6bdB569xm6r1/J5/Thc2XWPDHglfyDOlA+EHHPTfclu6TX3eS1GMhVvdl4yWESumVYQHswqrY9QakHmJzaSZkBlDD/hbqUeLXLcnUdki7l6gYh77H627ABCsoLzhXsS5IQD6ZWeBEchMQTtlFKbzlc7PEiP0S8EL+1odI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707323826; c=relaxed/simple; bh=CmINp01hMiGNdMbBKNDomNPzEGS4e5sFEub3Hb4c8F8=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=v2mH1mY8IkhOYZqX3uYGrJ+nMOZrtXkD+/aqIjHyu4eeaZyldRljM5aWW+DoUmRjLa6Jj+cA80miT+eZYNE5u/rTHKJzVnpEpNXE/u12x/3WHlD4B3JFbda+MQYGoXS9I8Sp2IHWpirofjBRcdYHnfqrS2P13jUCd7yvWUp9MTg= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1707323821; h=from:from: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=/bXnuDNaTqixX6GGJxe+ACVhzvNnL/+eFuS7QIiy6Ik=; b=EzqgcD4mX/5XYF3/Rd7Cl88ntdOMd2nTUGDgksLjYmh5uJMK+Yv1yrUmQH1vULCUKna9mx Qm5/pQxGPDuXtXo56sLJgCTqwBiuhz4aA3jGm5jWqlSHCJNM/wlDWZxZO8/hAahJ8/5A58 ss2meRk2ekgacLqNudVaGC6o7w+N74Y= Received: from mail-yw1-f198.google.com (mail-yw1-f198.google.com [209.85.128.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-510-4uJIdxgoP62pImlxrzDNEQ-1; Wed, 07 Feb 2024 11:37:00 -0500 X-MC-Unique: 4uJIdxgoP62pImlxrzDNEQ-1 Received: by mail-yw1-f198.google.com with SMTP id 00721157ae682-60481a6b1b5so15778447b3.3 for ; Wed, 07 Feb 2024 08:37:00 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707323819; x=1707928619; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=/bXnuDNaTqixX6GGJxe+ACVhzvNnL/+eFuS7QIiy6Ik=; b=V8+DBRRA9u8FYnlv7EsdqeYredUyuNZnDD0tuhaVyCWYXVRX665xZ7BZtfz5XaeBaL sgnMmsEwTAe+rc+F8O8xMomI8b/hbtfEaftow5Tx1RY3Dtf66vhAXqf2jntTKA8eyWMM uw34I3AroMoPLaLh3YBk91l86iEZG4UTcAo0xu5Dcpg1m5+k9cjHkVayMGuZqvKIY+07 Dz/w2Ai6LT14DGHqOu36z5LEW1wd7444sVQCk+DgM3LSXxdvGF3R6kzPteBD5KSaHxFP PiAmG61Qm+/Yi+0J/TSAsqxIMzvXhikcAAsg0BYsSAAP2muOSciFctaAb2SqhW74DGVk hVLg== X-Forwarded-Encrypted: i=1; AJvYcCW9ibAnVFIxJvGp2oDIQAq48w1SbuQi1b3zxbjIIOOEZ3Nu/0mixEBBTWJ6Qrl4LSKCPne9u7Hg6HqR/DD4I7YIo3kCss0= X-Gm-Message-State: AOJu0YxA9t/K7qCyWlnzx0N44um6DxBG4UYV056DPuLGaMxkVy3c+eCe xg/U+Rl2i7dHIJJCAe10j+YK4Y9hv5TOeD2KJ0nvuFiWoh1Ao8aWafs+/9+o/h71EPRAPDcyzuh krjgr9L1vN8jtqgG+IiNum8B75JtviHbXIz2P5hliZ/rxvZKiJ0MSzSytxisxzXKxCyWaRxAz0C Pb7JSvkTlaRQW+iYXuTb7CLthhQRs= X-Received: by 2002:a81:ad62:0:b0:5ff:a8da:5a5a with SMTP id l34-20020a81ad62000000b005ffa8da5a5amr5256821ywk.5.1707323819593; Wed, 07 Feb 2024 08:36:59 -0800 (PST) X-Google-Smtp-Source: AGHT+IFbjC8TrvBbDCiVgH6Qs76/d/RND+7jFHEQoN+QUGok9sgfiBKd9y4EGymYvxdeT71sdNgIc3k0ULZ7t2jmcJU= X-Received: by 2002:a81:ad62:0:b0:5ff:a8da:5a5a with SMTP id l34-20020a81ad62000000b005ffa8da5a5amr5256799ywk.5.1707323819047; Wed, 07 Feb 2024 08:36:59 -0800 (PST) MIME-Version: 1.0 References: <275f454b-537a-44d2-8aae-85e4484d9c52@foss.st.com> In-Reply-To: <275f454b-537a-44d2-8aae-85e4484d9c52@foss.st.com> From: Jonathan Wakely Date: Wed, 7 Feb 2024 16:36:43 +0000 Message-ID: Subject: Re: [PATCH v2] libstdc++: optimize bit iterators assuming normalization [PR110807] To: Torbjorn SVENSSON Cc: Jonathan Wakely , Alexandre Oliva , gcc-patches , "libstdc++" , Yvan Roux X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/alternative; boundary="000000000000c04dfa0610cd4d50" X-Spam-Status: No, score=-6.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,HTML_MESSAGE,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE,URI_HEX 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: --000000000000c04dfa0610cd4d50 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 7 Feb 2024 at 16:25, Torbjorn SVENSSON < torbjorn.svensson@foss.st.com> wrote: > Hi, > > Is it okay to backport e39b3e02c27bd771a07e385f9672ecf1a45ced77 to > releases/gcc-13? > It would also need 807f47497f17ed50be91f0f879308cb6fa063966 Please test with that as well, and OK for both if all goes well. > Without this backport, I see this failure on arm-none-eabi: > > FAIL: 23_containers/vector/bool/110807.cc (test for excess errors) > > Kind regards, > Torbj=C3=B6rn > > > On 2023-11-09 02:22, Jonathan Wakely wrote: > > > > > > On Thu, 9 Nov 2023, 01:17 Alexandre Oliva, > > wrote: > > > > On Nov 8, 2023, Jonathan Wakely > > wrote: > > > > > A single underscore prefix on __GLIBCXX_BUILTIN_ASSUME and > > > __GLIBCXX_DISABLE_ASSUMPTIONS please. > > > > That's entirely gone now. > > > > >> + do \ > > >> + if (std::is_constant_evaluated ()) \ > > >> + static_assert(expr); \ > > > > > This can never be valid. > > > > *nod* > > > > > This already works fine in constant evaluation anyway. > > > > Yeah, that's what I figured. > > > > > But what's the null dereference for? > > > > The idea was to clearly trigger undefined behavior. Maybe it wasn't > > needed, it didn't occur to me that __builtin_unreachable() would be > > enough. I realize I was really trying to emulate attribute assume, > even > > without knowing it existed ;-) > > > > >> +#define __GLIBCXX_BUILTIN_ASSUME(expr) \ > > >> + (void)(false && (expr)) > > > > > What's the point of this, just to verify that (expr) is > contextually > > > convertible to bool? > > > > I'd have phrased it as "avoid the case in which something compiles > with > > -O0 but not with -O", but yeah ;-) > > > > > We don't use the _p suffix for predicates in the library. > > > Please use just _M_normalized or _M_is_normalized. > > > > ACK. It's also gone now. > > > > > But do we even need this function? It's not used anywhere else, > > can we > > > just inline the condition into _M_assume_normalized() ? > > > > I had other uses for it in earlier versions of the patch, but it > makes > > no sense any more indeed. > > > > >> + _GLIBCXX20_CONSTEXPR > > >> + void > > >> + _M_assume_normalized() const > > > > > I think this should use _GLIBCXX_ALWAYS_INLINE > > > > *nod*, thanks > > > > >> + { > > >> + __GLIBCXX_BUILTIN_ASSUME (_M_normalized_p ()); > > > > > Is there even any benefit to this macro? > > > > I just thought it could have other uses, without being aware that t= he > > entire concept was available as a statement attribute. Funny, I'd > even > > searched for it among the existing attributes and builtins, but > somehow > > I managed to miss it. Thanks for getting me back down that path. > > > > > __attribute__((__assume__(_M_offset < > > unsigned(_S_word_bit)))); > > > > That unfortunately doesn't work, because the assume lowering doesn't > go > > as far as dereferencing the implicit this and making an SSA_NAME out > of > > the loaded _M_offset, which we'd need to be able to optimize based = on > > it. But that only took me a while to figure out and massage into > > something that had the desired effect. Now, maybe the above *shoul= d* > > have that effect already, but unfortunately it doesn't. > > > > > Maybe even get rid of _M_assume_normalized() as a function and > just > > > put that attribute everywhere you currently use > _M_assume_normalized. > > > > Because of the slight kludge required to make the attribute have the > > desired effect (namely ensuring the _M_offset reference is > evaluated), > > I've retained it as an inline function. > > > > Here's what I'm retesting now. WDYT? > > > > > > ofst needs to be __ofst but OK for trunk with that change. > > > > We probably want this on the gcc-13 branch too, but let's give it some > > time on trunk in case the assume attribute isn't quite ready for prime > time. > > --000000000000c04dfa0610cd4d50--