From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id 8864B3858430 for ; Tue, 16 Nov 2021 12:06:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8864B3858430 Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id 6C3BE1FD26; Tue, 16 Nov 2021 12:06:54 +0000 (UTC) Received: from murzim.suse.de (murzim.suse.de [10.160.4.192]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 63FF3A3B81; Tue, 16 Nov 2021 12:06:54 +0000 (UTC) Date: Tue, 16 Nov 2021 13:06:54 +0100 (CET) From: Richard Biener To: Jan Hubicka cc: gcc-patches@gcc.gnu.org Subject: Re: Use modref kills in tree-ssa-dse In-Reply-To: <20211114232310.GD42690@kam.mff.cuni.cz> Message-ID: <46408584-8380-346n-2034-n945no4q9878@fhfr.qr> References: <20211114232310.GD42690@kam.mff.cuni.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-11.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP 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: Tue, 16 Nov 2021 12:06:58 -0000 On Mon, 15 Nov 2021, Jan Hubicka wrote: > Hi, > this patch extends tree-ssa-dse to use modref kill summary to clear > live_bytes. This makes it possible to remove calls that are killed > in parts. > > I noticed that DSE duplicates the logic of tree-ssa-alias that is > mathing bases of memory accesses. Here operands_equal_p (base1, base, > OEP_ADDRESS_OF) is used. So it won't work with mismatching memref > offsets. We probably want to commonize this and add common function > that matches bases and returns offset adjustments. I wonder however if > it can catch any cases that the tree-ssa-alias code doesn't? Not sure, tree-ssa-dse.c doesn't seem to handle MEM_REF with offset? VN has adjust_offsets_for_equal_base_address for this purpose. I agree that some common functionality like bool get_relative_extent_of (const ao_ref *base, const ao_ref *ref, poly_int64 *offset); that computes [offset, offset + ref->[max_]size] of REF adjusted as to make ao_ref_base have the same address (or return false if not possible). Then [ base->offset, base->offset + base->max_size ] can be compared against that. > Other check that stmt_kills_ref_p has and tree-ssa-dse is for > non-call-exceptions. > > Bootstrapped/regtested x86_64-linux, OK? See below. > gcc/ChangeLog: > > * ipa-modref.c (get_modref_function_summary): New function. > * ipa-modref.h (get_modref_function_summary): Declare. > * tree-ssa-dse.c (clear_live_bytes_for_ref): Break out from ... > (clear_bytes_written_by): ... here; add handling of modref summary. > > gcc/testsuite/ChangeLog: > > * gcc.dg/tree-ssa/modref-dse-4.c: New test. > > diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c > index df4612bbff9..8966f9fd2a4 100644 > --- a/gcc/ipa-modref.c > +++ b/gcc/ipa-modref.c > @@ -724,6 +724,22 @@ get_modref_function_summary (cgraph_node *func) > return r; > } > > +/* Get function summary for CALL if it exists, return NULL otherwise. > + If INTERPOSED is non-NULL set it to true if call may be interposed. */ > + > +modref_summary * > +get_modref_function_summary (gcall *call, bool *interposed) > +{ > + tree callee = gimple_call_fndecl (call); > + if (!callee) > + return NULL; > + struct cgraph_node *node = cgraph_node::get (callee); > + if (!node) > + return NULL; > + if (interposed) > + *interposed = !node->binds_to_current_def_p (); > + return get_modref_function_summary (node); > +} > + > namespace { > > /* Construct modref_access_node from REF. */ > diff --git a/gcc/ipa-modref.h b/gcc/ipa-modref.h > index 9e8a30fd80a..72e608864ce 100644 > --- a/gcc/ipa-modref.h > +++ b/gcc/ipa-modref.h > @@ -50,6 +50,7 @@ struct GTY(()) modref_summary > }; > > modref_summary *get_modref_function_summary (cgraph_node *func); > +modref_summary *get_modref_function_summary (gcall *call, bool *interposed); > void ipa_modref_c_finalize (); > void ipa_merge_modref_summary_after_inlining (cgraph_edge *e); > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/modref-dse-4.c b/gcc/testsuite/gcc.dg/tree-ssa/modref-dse-4.c > new file mode 100644 > index 00000000000..81aa7dc587c > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/modref-dse-4.c > @@ -0,0 +1,26 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-tree-dse2-details" } */ > +struct a {int a,b,c;}; > +__attribute__ ((noinline)) > +void > +kill_me (struct a *a) > +{ > + a->a=0; > + a->b=0; > + a->c=0; > +} > +__attribute__ ((noinline)) > +void > +my_pleasure (struct a *a) > +{ > + a->a=1; > + a->c=2; > +} > +void > +set (struct a *a) > +{ > + kill_me (a); > + my_pleasure (a); > + a->b=1; > +} > +/* { dg-final { scan-tree-dump "Deleted dead store: kill_me" "dse2" } } */ > diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c > index ce0083a6dab..d2f54b0faad 100644 > --- a/gcc/tree-ssa-dse.c > +++ b/gcc/tree-ssa-dse.c > @@ -209,6 +209,24 @@ normalize_ref (ao_ref *copy, ao_ref *ref) > return true; > } > > +/* Update LIVE_BYTES tracking REF for write to WRITE: > + Verify we have the same base memory address, the write > + has a known size and overlaps with REF. */ > +static void > +clear_live_bytes_for_ref (sbitmap live_bytes, ao_ref *ref, ao_ref *write) > +{ > + HOST_WIDE_INT start, size; > + > + if (valid_ao_ref_for_dse (write) > + && operand_equal_p (write->base, ref->base, OEP_ADDRESS_OF) > + && known_eq (write->size, write->max_size) > + && normalize_ref (write, ref) normalize_ref alters 'write', I think we should work on a local copy here. See live_bytes_read which takes a copy of 'use_ref'. Otherwise looks good to me. Thanks, Richard. > + && (write->offset - ref->offset).is_constant (&start) > + && write->size.is_constant (&size)) > + bitmap_clear_range (live_bytes, start / BITS_PER_UNIT, > + size / BITS_PER_UNIT); > +} > + > /* Clear any bytes written by STMT from the bitmap LIVE_BYTES. The base > address written by STMT must match the one found in REF, which must > have its base address previously initialized. > @@ -220,20 +238,21 @@ static void > clear_bytes_written_by (sbitmap live_bytes, gimple *stmt, ao_ref *ref) > { > ao_ref write; > + > + if (gcall *call = dyn_cast (stmt)) > + { > + bool interposed; > + modref_summary *summary = get_modref_function_summary (call, &interposed); > + > + if (summary && !interposed) > + for (auto kill : summary->kills) > + if (kill.get_ao_ref (as_a (stmt), &write)) > + clear_live_bytes_for_ref (live_bytes, ref, &write); > + } > if (!initialize_ao_ref_for_dse (stmt, &write)) > return; > > - /* Verify we have the same base memory address, the write > - has a known size and overlaps with REF. */ > - HOST_WIDE_INT start, size; > - if (valid_ao_ref_for_dse (&write) > - && operand_equal_p (write.base, ref->base, OEP_ADDRESS_OF) > - && known_eq (write.size, write.max_size) > - && normalize_ref (&write, ref) > - && (write.offset - ref->offset).is_constant (&start) > - && write.size.is_constant (&size)) > - bitmap_clear_range (live_bytes, start / BITS_PER_UNIT, > - size / BITS_PER_UNIT); > + clear_live_bytes_for_ref (live_bytes, ref, &write); > } > > /* REF is a memory write. Extract relevant information from it and > -- Richard Biener SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)