On Tue, 13 Oct 2020, Jan Hubicka wrote:
> > > So I implemented my own pattern matching and I think we should switch
> > > ipa-prop
> > > on it incrementally. It is based on similar logic in
> > > ao_ref_init_from_ptr_and_size.
> >
> > So instead of re-inventing the wheel what about splitting out a
> > common helper instead?
>
> It was my original idea. However it is not completely trivial:
> ao_ref_init_from_ptr_and_size does bit something else since it is trying
> to keep the original ref inside ADDR_EXPR (if one is found) to feed it
> into oracle rather then stripping them all. If refs are present it does
> not need to build MEM_REF.
You've also coded other subtle differences - is it really better
to have p_1 with unknown offset for in p_3 = p_1 + i_2 instead of
having p_3? I suppose the idea is to look for an offsetted
default def (aka parameter)?
Frankly I don't like the loop - unbounded walk over unoptimized ptr += 4;
ptr += 4; ... chain, possibly quadratic if it's like
void foo (void *p, int i)
{
void *p1 = p + i;
void *p2 = p1 + i;
void *p3 = p2 + i;
bar (p1, p2, p3);
}
so I wonder if the propagation engine instead wants to cache
the discovered base + offset for a (pointer?) SSA name? Not
sure if this recursion when the offset becomes unknown is
of any help to ipa-prop - IPA prop is also interested in
other than pointer parameters I guess.
> static void
> ao_ref_init_from_ptr_and_range (ao_ref *ref, tree ptr,
> bool range_known,
> poly_int64 offset,
> poly_int64 size,
> poly_int64 max_size)
> {
> poly_int64 t, extra_offset = 0;
>
> ref->ref = NULL_TREE;
> if (TREE_CODE (ptr) == SSA_NAME)
> {
> gimple *stmt = SSA_NAME_DEF_STMT (ptr);
> if (gimple_assign_single_p (stmt)
> && gimple_assign_rhs_code (stmt) == ADDR_EXPR)
> ptr = gimple_assign_rhs1 (stmt);
> else if (is_gimple_assign (stmt)
> && gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR
> && ptrdiff_tree_p (gimple_assign_rhs2 (stmt), &extra_offset))
> {
> ptr = gimple_assign_rhs1 (stmt);
> extra_offset *= BITS_PER_UNIT;
> }
> }
>
> if (TREE_CODE (ptr) == ADDR_EXPR)
> {
> ref->base = get_addr_base_and_unit_offset (TREE_OPERAND (ptr, 0), &t);
> if (ref->base)
> ref->offset = BITS_PER_UNIT * t;
> else
> {
> range_known = false;
> ref->offset = 0;
> ref->base = get_base_address (TREE_OPERAND (ptr, 0));
> }
> }
> else
> {
> gcc_assert (POINTER_TYPE_P (TREE_TYPE (ptr)));
> ref->base = build2 (MEM_REF, char_type_node,
> ptr, null_pointer_node);
> ref->offset = 0;
> }
>
> I could add a flag if I am looking for ADDR_EXPR or just the base
> pointer but it seemed more for incremental change. The new helper
> should be immediately useful for ipa-modref, ipa-prop and
> ipa-polymorphic-call though.
>
> Honza
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend
