> 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)?
Yes this was intentional (for modref). Modref primarily cares about the based pointer (for PTA). if it knows offset it is better. > > 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. I would like to extend ipa-prop to also record anscessor jump functions without offset known (to help modref propagate), so in both cases this should work. ipa-prop currently handles arith/simple and unary apass through separately and only then it does the ancessor. Concerning the loop, maybe stopping after few iterations is better solution, since ealry pases should eliminate the chains, but I can add the cache. Honza > > > 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 <rguent...@suse.de> > SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, > Germany; GF: Felix Imend