From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x52c.google.com (mail-ed1-x52c.google.com [IPv6:2a00:1450:4864:20::52c]) by sourceware.org (Postfix) with ESMTPS id 8E30C385840A for ; Thu, 11 Nov 2021 12:32:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8E30C385840A Received: by mail-ed1-x52c.google.com with SMTP id x15so23662780edv.1 for ; Thu, 11 Nov 2021 04:32:40 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Y1GrpcT+LPm+v74HDjznn41Cx+JvDTMzBvodWW680RA=; b=BLmEaEf2yiwDXcTjHKId0Jw5R06L3sQjSVnrU9TvzE9TJaGnfniZZBum83dAtjL8dd 1qCS2E35qYCtRbSey7FjD7SA5blljuIvgsRec1uQE/JNcUNZ6gvW6Jis2lRyMEFAiGH3 c7wWGci7XikoA7oeVKJ7t4lZIiOClpm+l+NebE8pY+hvnp0MZb8QI/B1h3wZv3FO5VbC PCgs+b+G40eCdD6aYQsafkP5ud0OFHJIVfKFInQPmbWQDbrLhEj4HKMbCNkIdaYC1/Bf o927tv7pLXevFBua6Q8YOiiFafQ6Ni/gRZvexgyq0QlM3cG3hwyKkve2J/F9tfwRfpuV GNIQ== X-Gm-Message-State: AOAM530eL5kUPOu/pbqKoV0nidFORfAeSkshoCENTJT0yi11rj62QYnO MyTkD+icq+UZ3j6IoDQt6XVOCCH9zH97UYWYZ4M= X-Google-Smtp-Source: ABdhPJxsznTvJLp2GwhY+B8LkbISx3lZtZyVVX1AeK+2qTQZ9C9noer7p/rGG5rvdGCOtRs+/MzeqLt32tX+tNnOaZ4= X-Received: by 2002:a17:906:c301:: with SMTP id s1mr8828548ejz.56.1636633959370; Thu, 11 Nov 2021 04:32:39 -0800 (PST) MIME-Version: 1.0 References: <20211110124316.GF97553@kam.mff.cuni.cz> <20211111120715.GC47134@kam.mff.cuni.cz> In-Reply-To: <20211111120715.GC47134@kam.mff.cuni.cz> From: Richard Biener Date: Thu, 11 Nov 2021 13:32:28 +0100 Message-ID: Subject: Re: Use modref summary to DSE calls to non-pure functions To: Jan Hubicka Cc: GCC Patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, 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: Thu, 11 Nov 2021 12:32:42 -0000 On Thu, Nov 11, 2021 at 1:07 PM Jan Hubicka wrote: > > > > + /* Unlike alias oracle we can not skip subtrees based on TBAA check. > > > + Count the size of the whole tree to verify that we will not need too many > > > + tests. */ > > > + FOR_EACH_VEC_SAFE_ELT (summary->stores->bases, i, base_node) > > > + FOR_EACH_VEC_SAFE_ELT (base_node->refs, j, ref_node) > > > + FOR_EACH_VEC_SAFE_ELT (ref_node->accesses, k, access_node) > > > + if (num_tests++ > max_tests) > > > + return false; > > > > at least the innermost loop can be done as > > > > if (num_tests += ref_node->accesses.length () > max_tests) > > > > no? > > Yep that was stupid, sorry for that ;)) > > > > > + > > > + /* Walk all memory writes and verify that they are dead. */ > > > + FOR_EACH_VEC_SAFE_ELT (summary->stores->bases, i, base_node) > > > + FOR_EACH_VEC_SAFE_ELT (base_node->refs, j, ref_node) > > > + FOR_EACH_VEC_SAFE_ELT (ref_node->accesses, k, access_node) > > > + { > > > + /* ??? if offset is unkonwn it may be negative. Not sure > > > + how to construct ref here. */ > > > > I think you can't, you could use -poly_int64_max or so. > > I need a ref to give to dse_classify_store. It needs base to track live > bytes etc which is not very useful if I do not know the range. However > DSE is still useful since I can hit free or end of lifetime of the decl. > I was wondering if I should simply implement a lightweight version of > dse_clasify_store that handles this case? No, I think if it turns out useful then we want a way to have such ref represented by an ao_ref. Note that when we come from a ref tree we know handled-components only will increase offset, only the base MEM_REF can contain a pointer subtraction (but the result of that is the base then). In what cases does parm_offset_known end up false? Is that when seeing a POINTER_PLUS_EXPR with unknown offset? So yes, that's a case we cannot capture right now - the only thing that remains is a pointer with a known points-to-set - a similar problem as with the pure call PRE. You could in theory allocate a scratch SSA name and attach points-to-info to it. And when the call argument is &decl based then you could set offset to zero. > > > > > + if (!access_node->parm_offset_known) > > > + return false; > > > > But you could do this check in the loop computing num_tests ... > > (we could also cache the count and whether any of the refs have unknown offset > > in the summary?) > > Yep, I plan to add cache for bits like this (and the check for accessing > global memory). Just want to push bit more of the cleanups I have in my > local tree. > > > > > + tree arg; > > > + if (access_node->parm_index == MODREF_STATIC_CHAIN_PARM) > > > + arg = gimple_call_chain (stmt); > > > + else > > > + arg = gimple_call_arg (stmt, access_node->parm_index); > > > + > > > + ao_ref ref; > > > + poly_offset_int off = (poly_offset_int)access_node->offset > > > + + ((poly_offset_int)access_node->parm_offset > > > + << LOG2_BITS_PER_UNIT); > > > + poly_int64 off2; > > > + if (!off.to_shwi (&off2)) > > > + return false; > > > + ao_ref_init_from_ptr_and_range > > > + (&ref, arg, true, off2, access_node->size, > > > + access_node->max_size); > > > + ref.ref_alias_set = ref_node->ref; > > > + ref.base_alias_set = base_node->base; > > > + > > > + bool byte_tracking_enabled > > > + = setup_live_bytes_from_ref (&ref, live_bytes); > > > + enum dse_store_status store_status; > > > + > > > + store_status = dse_classify_store (&ref, stmt, > > > + byte_tracking_enabled, > > > + live_bytes, &by_clobber_p); > > > + if (store_status != DSE_STORE_DEAD) > > > + return false; > > > + } > > > + /* Check also value stored by the call. */ > > > + if (gimple_store_p (stmt)) > > > + { > > > + ao_ref ref; > > > + > > > + if (!initialize_ao_ref_for_dse (stmt, &ref)) > > > + gcc_unreachable (); > > > + bool byte_tracking_enabled > > > + = setup_live_bytes_from_ref (&ref, live_bytes); > > > + enum dse_store_status store_status; > > > + > > > + store_status = dse_classify_store (&ref, stmt, > > > + byte_tracking_enabled, > > > + live_bytes, &by_clobber_p); > > > + if (store_status != DSE_STORE_DEAD) > > > + return false; > > > + } > > > + delete_dead_or_redundant_assignment (gsi, "dead", need_eh_cleanup); > > > + return true; > > > +} > > > + > > > namespace { > > > > > > const pass_data pass_data_dse = > > > @@ -1235,7 +1363,14 @@ pass_dse::execute (function *fun) > > > gimple *stmt = gsi_stmt (gsi); > > > > > > if (gimple_vdef (stmt)) > > > - dse_optimize_stmt (fun, &gsi, live_bytes); > > > + { > > > + gcall *call = dyn_cast (stmt); > > > + > > > + if (call && dse_optimize_call (&gsi, live_bytes)) > > > + /* We removed a dead call. */; > > > + else > > > + dse_optimize_store (fun, &gsi, live_bytes); > > > > I think we want to refactor both functions, dse_optimize_stmt has some > > early outs that apply generally, and it handles some builtin calls > > that we don't want to re-handle with dse_optimize_call. > > > > So I wonder if it is either possible to call the new function from > > inside dse_optimize_stmt instead, after we handled the return > > value of call for example or different refactoring can make the flow > > more obvious. > > It was my initial plan. However I was not sure how much I would get from > that. > > The function starts with: > > /* Don't return early on *this_2(D) ={v} {CLOBBER}. */ > if (gimple_has_volatile_ops (stmt) > && (!gimple_clobber_p (stmt) > || TREE_CODE (gimple_assign_lhs (stmt)) != MEM_REF)) > return; > > ao_ref ref; > if (!initialize_ao_ref_for_dse (stmt, &ref)) > return; > > The check about clobber does not apply to calls and then it gives up on > functions not returning aggregates (that is a common case). > > For functions returing aggregates it tries to prove that retval is dead > and replace it. > > I guess I can simply call my analysis from the second return above and > from the code removing dead LHS call instead of doing it from the main > walker and drop the LHS handling? Yeah, something like that. Richard. > Thank you, > Honza > > > > Thanks, > > Richard. > > > > > + } > > > else if (def_operand_p > > > def_p = single_ssa_def_operand (stmt, SSA_OP_DEF)) > > > {