On Thu, Nov 11, 2021 at 1:07 PM Jan Hubicka <hubi...@kam.mff.cuni.cz> 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 <gcall *> (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)) > > > {