On Wed, Mar 16, 2016 at 12:20 PM, Richard Biener <richard.guent...@gmail.com> wrote: > > On Wed, Mar 16, 2016 at 10:59 AM, Bin Cheng <bin.ch...@arm.com> wrote: > > Hi, > > ...... > > Bootstrap and test on x86_64 and AArch64. Is it OK, not sure if it's GCC 7? > > Hmm. Hi, Thanks for reviewing. > > + equal_p = true; > + if (e1->base_address && e2->base_address) > + equal_p &= operand_equal_p (e1->base_address, e2->base_address, 0); > + if (e1->offset && e2->offset) > + equal_p &= operand_equal_p (e1->offset, e2->offset, 0); > > surely better to return false early. > > I think we don't want this in tree-data-refs.h also because of ... > > @@ -615,15 +619,29 @@ > hash_memrefs_baserefs_and_store_DRs_read_written_info > (data_reference_p a) > data_reference_p *master_dr, *base_master_dr;and REALPART) before creating > the DR (or adjust the equality function and hashing > tree ref = DR_REF (a); > tree base_ref = DR_BASE_OBJECT (a); > + innermost_loop_behavior *innermost = &DR_INNERMOST (a); > tree ca = bb_predicate (gimple_bb (DR_STMT (a))); > bool exist1, exist2; > > - while (TREE_CODE (ref) == COMPONENT_REF > - || TREE_CODE (ref) == IMAGPART_EXPR > - || TREE_CODE (ref) == REALPART_EXPR) > - ref = TREE_OPERAND (ref, 0); > + /* If reference in DR has innermost loop behavior and it is not > + a compound memory reference, we store it to innermost_DR_map, > + otherwise to ref_DR_map. */ > + if (TREE_CODE (ref) == COMPONENT_REF > + || TREE_CODE (ref) == IMAGPART_EXPR > + || TREE_CODE (ref) == REALPART_EXPR > + || !(DR_BASE_ADDRESS (a) || DR_OFFSET (a) > + || DR_INIT (a) || DR_STEP (a) || DR_ALIGNED_TO (a))) > + { > + while (TREE_CODE (ref) == COMPONENT_REF > + || TREE_CODE (ref) == IMAGPART_EXPR > + || TREE_CODE (ref) == REALPART_EXPR) > + ref = TREE_OPERAND (ref, 0); > + > + master_dr = &ref_DR_map->get_or_insert (ref, &exist1); > + } > + else > + master_dr = &innermost_DR_map->get_or_insert (innermost, &exist1); > > we don't want an extra hashmap but replace ref_DR_map entirely. So we'd need > to > strip outermost non-variant handled-components (COMPONENT_REF, IMAGPART > and REALPART) before creating the DR (or adjust the equality function > and hashing > to disregard them which means subtracting their offset from DR_INIT. I am not sure if I understand correctly. But for component reference, it is the base object that we want to record/track. For example,
for (i = 0; i < N; i++) { m = *data++; m1 = p1->x - m; m2 = p2->x + m; p3->y = (m1 >= m2) ? p1->y : p2->y; p1++; p2++; p3++; } We want to infer that reads of p1/p2 in condition statement won't trap because there are unconditional reads of the structures, though the unconditional reads are actual of other sub-objects. Here it is the invariant part of address that we want to track. Also illustrated by this example, we can't rely on data-ref analyzer here. Because in gathering/scattering cases, the address could be not affine at all. > > To adjust the references we collect you'd maybe could use a callback > to get_references_in_stmt > to adjust them. > > OTOH post-processing the DRs in if_convertible_loop_p_1 can be as simple as Is this a part of the method you suggested above, or is it an alternative one? If it's the latter, then I have below questions embedded. > > Index: tree-if-conv.c > =================================================================== > --- tree-if-conv.c (revision 234215) > +++ tree-if-conv.c (working copy) > @@ -1235,6 +1220,38 @@ if_convertible_loop_p_1 (struct loop *lo > > for (i = 0; refs->iterate (i, &dr); i++) > { > + tree *refp = &DR_REF (dr); > + while ((TREE_CODE (*refp) == COMPONENT_REF > + && TREE_OPERAND (*refp, 2) == NULL_TREE) > + || TREE_CODE (*refp) == IMAGPART_EXPR > + || TREE_CODE (*refp) == REALPART_EXPR) > + refp = &TREE_OPERAND (*refp, 0); > + if (refp != &DR_REF (dr)) > + { > + tree saved_base = *refp; > + *refp = integer_zero_node; > + > + if (DR_INIT (dr)) > + { > + tree poffset; > + int punsignedp, preversep, pvolatilep; > + machine_mode pmode; > + HOST_WIDE_INT pbitsize, pbitpos; > + get_inner_reference (DR_REF (dr), &pbitsize, &pbitpos, &poffset, > + &pmode, &punsignedp, &preversep, > &pvolatilep, > + false); > + gcc_assert (poffset == NULL_TREE); > + > + DR_INIT (dr) > + = wide_int_to_tree (ssizetype, > + wi::sub (DR_INIT (dr), > + pbitpos / BITS_PER_UNIT)); > + } > + > + *refp = saved_base; > + DR_REF (dr) = *refp; > + } Looks to me the code is trying to resolve difference between two (or more) component references, which is DR_INIT in the code. But DR_INIT is not the only thing needs to be handled. For a structure containing two sub-arrays, DR_OFFSET may be different too. Actually I did try to replace ref_DR_map. For memory reference doesn't have innermost affine behavior, we need to modify DR fields by populating it with artificial data. This results in some kind of over-designed code. IMHO, modifying some DR structures outside of data-ref analyzer isn't that good. After comparing the two methods, I think may be this one is better, of course with the cost of two different maps. I might be misunderstanding your idea, so please correct if I was wrong. Thanks, bin > + > dr->aux = XNEW (struct ifc_dr); > DR_BASE_W_UNCONDITIONALLY (dr) = false; > DR_RW_UNCONDITIONALLY (dr) = false; > > > Can you add a testcase for the vectorization? > > Richard. >