On Wed, Mar 16, 2016 at 12:20 PM, Richard Biener
<[email protected]> wrote:
>
> On Wed, Mar 16, 2016 at 10:59 AM, Bin Cheng <[email protected]> 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.
>
