From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2001:67c:2178:6::1d]) by sourceware.org (Postfix) with ESMTPS id C11D4385703C for ; Mon, 29 May 2023 14:51:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C11D4385703C Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=suse.cz Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id EFE661F88E; Mon, 29 May 2023 14:51:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1685371876; h=from:from:reply-to: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=8RaosyL7Y8ssbquQ6f4f7V95Tc1dGUzlPt9UufYpXjk=; b=EOrA8HjoFgrPDtnMAaNgSE1qPvlBrtgfwIFLqnDKV/9RlnYxeZmXHSSF91fDXnJy2ikxx1 tWnOj0hW5xoeRcEevrIkcY7x30w05q7TNj4+k7key6IEzadpLVsVfjv+l8L37vbnsPNdNV anixNkz3fPWGD8xGDnNQbK4tZG97eQ0= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1685371876; h=from:from:reply-to: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=8RaosyL7Y8ssbquQ6f4f7V95Tc1dGUzlPt9UufYpXjk=; b=W3Urchq7Q6+JMSYhiMDPEJCmoZexiso3D8rQScQMK+F/L9Tr7ivbmDcP3SlTQxGt8+hlXN SqhPKrV3DihWNRDg== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id E55E91332D; Mon, 29 May 2023 14:51:16 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id J+bzN+S7dGTRPAAAMHmgww (envelope-from ); Mon, 29 May 2023 14:51:16 +0000 From: Martin Jambor To: GCC patches Cc: Andrew MacLeod , Aldy Hernandez Subject: Re: [PATCH] Implement ipa_vr hashing. In-Reply-To: <20230522185622.537454-2-aldyh@redhat.com> References: <20230522185622.537454-1-aldyh@redhat.com> <20230522185622.537454-2-aldyh@redhat.com> User-Agent: Notmuch/0.37 (https://notmuchmail.org) Emacs/28.2 (x86_64-suse-linux-gnu) Date: Mon, 29 May 2023 16:51:16 +0200 Message-ID: MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-11.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,SPF_HELO_NONE,SPF_SOFTFAIL,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: Hi, On Mon, May 22 2023, Aldy Hernandez via Gcc-patches wrote: > Implement hashing for ipa_vr. When all is said and done, all these > patches incurr a 7.64% slowdown for ipa-cp, with is entirely covered by > the similar 7% increase in this area last week. So we get type agnostic > ranges with "infinite" range precision close to free. Do you know why/where this slow-down happens? Do we perhaps want to limit the "infiniteness" a little somehow? Also, jump functions live for a long time, have you looked at how memory hungry they become? I hope that the hashing would be good at preventing any issues. Generally, I think I OK with the patches if the impact on memory is not too bad, though I guess they depend on the one I looked at last week, so we may focus on that one first. Thanks, Martin > > There is no change in overall compilation. > > OK? > > gcc/ChangeLog: > > * ipa-prop.cc (struct ipa_vr_ggc_hash_traits): Adjust for use with > ipa_vr instead of value_range. > (gt_pch_nx): Same. > (gt_ggc_mx): Same. > (ipa_get_value_range): Same. > * value-range.cc (gt_pch_nx): Move to ipa-prop.cc and adjust for > ipa_vr. > (gt_ggc_mx): Same. > --- > gcc/ipa-prop.cc | 76 +++++++++++++++++++++++++++------------------- > gcc/value-range.cc | 15 --------- > 2 files changed, 45 insertions(+), 46 deletions(-) > > diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc > index c46a89f1b49..6383bc11e0a 100644 > --- a/gcc/ipa-prop.cc > +++ b/gcc/ipa-prop.cc > @@ -109,53 +109,53 @@ struct ipa_bit_ggc_hash_traits : public ggc_cache_remove > /* Hash table for avoid repeated allocations of equal ipa_bits. */ > static GTY ((cache)) hash_table *ipa_bits_hash_table; > > -/* Traits for a hash table for reusing value_ranges used for IPA. Note that > - the equiv bitmap is not hashed and is expected to be NULL. */ > +/* Traits for a hash table for reusing ranges. */ > > -struct ipa_vr_ggc_hash_traits : public ggc_cache_remove > +struct ipa_vr_ggc_hash_traits : public ggc_cache_remove > { > - typedef value_range *value_type; > - typedef value_range *compare_type; > + typedef ipa_vr *value_type; > + typedef const vrange *compare_type; > static hashval_t > - hash (const value_range *p) > + hash (const ipa_vr *p) > { > - tree min, max; > - value_range_kind kind = get_legacy_range (*p, min, max); > - inchash::hash hstate (kind); > - inchash::add_expr (min, hstate); > - inchash::add_expr (max, hstate); > + // This never get called, except in the verification code, as > + // ipa_get_value_range() calculates the hash itself. This > + // function is mostly here for completness' sake. > + Value_Range vr; > + p->get_vrange (vr); > + inchash::hash hstate; > + add_vrange (vr, hstate); > return hstate.end (); > } > static bool > - equal (const value_range *a, const value_range *b) > + equal (const ipa_vr *a, const vrange *b) > { > - return (types_compatible_p (a->type (), b->type ()) > - && *a == *b); > + return a->equal_p (*b); > } > static const bool empty_zero_p = true; > static void > - mark_empty (value_range *&p) > + mark_empty (ipa_vr *&p) > { > p = NULL; > } > static bool > - is_empty (const value_range *p) > + is_empty (const ipa_vr *p) > { > return p == NULL; > } > static bool > - is_deleted (const value_range *p) > + is_deleted (const ipa_vr *p) > { > - return p == reinterpret_cast (1); > + return p == reinterpret_cast (1); > } > static void > - mark_deleted (value_range *&p) > + mark_deleted (ipa_vr *&p) > { > - p = reinterpret_cast (1); > + p = reinterpret_cast (1); > } > }; > > -/* Hash table for avoid repeated allocations of equal value_ranges. */ > +/* Hash table for avoid repeated allocations of equal ranges. */ > static GTY ((cache)) hash_table *ipa_vr_hash_table; > > /* Holders of ipa cgraph hooks: */ > @@ -265,6 +265,22 @@ ipa_vr::dump (FILE *out) const > fprintf (out, "NO RANGE"); > } > > +// ?? These stubs are because we use an ipa_vr in a hash_traits and > +// hash-traits.h defines an extern of gt_ggc_mx (T &) instead of > +// picking up the gt_ggc_mx (T *) version. > +void > +gt_pch_nx (ipa_vr *&x) > +{ > + return gt_pch_nx ((ipa_vr *) x); > +} > + > +void > +gt_ggc_mx (ipa_vr *&x) > +{ > + return gt_ggc_mx ((ipa_vr *) x); > +} > + > + > /* Return true if DECL_FUNCTION_SPECIFIC_OPTIMIZATION of the decl associated > with NODE should prevent us from analyzing it for the purposes of IPA-CP. */ > > @@ -2284,27 +2300,25 @@ ipa_set_jfunc_bits (ipa_jump_func *jf, const widest_int &value, > jf->bits = ipa_get_ipa_bits_for_value (value, mask); > } > > -/* Return a pointer to a value_range just like *TMP, but either find it in > - ipa_vr_hash_table or allocate it in GC memory. TMP->equiv must be NULL. */ > +/* Return a pointer to an ipa_vr just like TMP, but either find it in > + ipa_vr_hash_table or allocate it in GC memory. */ > > static ipa_vr * > ipa_get_value_range (const vrange &tmp) > { > - /* FIXME: Add hashing support. > - value_range **slot = ipa_vr_hash_table->find_slot (tmp, INSERT); > + inchash::hash hstate; > + inchash::add_vrange (tmp, hstate); > + hashval_t hash = hstate.end (); > + ipa_vr **slot = ipa_vr_hash_table->find_slot_with_hash (&tmp, hash, INSERT); > if (*slot) > return *slot; > > - value_range *vr = new (ggc_alloc ()) value_range; > - *vr = *tmp; > - *slot = vr; > - */ > ipa_vr *vr = new (ggc_alloc ()) ipa_vr (tmp); > - > + *slot = vr; > return vr; > } > > -/* Assign to JF a pointer to a value_range just like TMP but either fetch a > +/* Assign to JF a pointer to a range just like TMP but either fetch a > copy from ipa_vr_hash_table or allocate a new on in GC memory. */ > > static void > diff --git a/gcc/value-range.cc b/gcc/value-range.cc > index 45b1e655967..7b81025357b 100644 > --- a/gcc/value-range.cc > +++ b/gcc/value-range.cc > @@ -2008,21 +2008,6 @@ gt_pch_nx (vrange *x, gt_pointer_operator op, void *cookie) > gcc_unreachable (); > } > > -// ?? These stubs are for ipa-prop.cc which use a value_range in a > -// hash_traits. hash-traits.h defines an extern of gt_ggc_mx (T &) > -// instead of picking up the gt_ggc_mx (T *) version. > -void > -gt_pch_nx (int_range<2> *&x) > -{ > - return gt_pch_nx ((irange *) x); > -} > - > -void > -gt_ggc_mx (int_range<2> *&x) > -{ > - return gt_ggc_mx ((irange *) x); > -} > - > #define DEFINE_INT_RANGE_INSTANCE(N) \ > template int_range::int_range(tree_node *, \ > const wide_int &, \ > -- > 2.40.1