Use modref summary to DSE calls to non-pure functions

Jan Hubicka hubicka@kam.mff.cuni.cz
Thu Nov 11 12:07:15 GMT 2021


> > +  /* 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?
> 
> > +         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?

Thank you,
Honza
> 
> Thanks,
> Richard.
> 
> > +           }
> >           else if (def_operand_p
> >                      def_p = single_ssa_def_operand (stmt, SSA_OP_DEF))
> >             {


More information about the Gcc-patches mailing list