Re: Tweak BB analysis for dr_analyze_innermost

["Bin.Cheng" <amker dot cheng at gmail dot com>]

On Wed, Jun 28, 2017 at 3:04 PM, Richard Sandiford
<richard.sandiford@linaro.org> wrote:
> "Bin.Cheng" <amker.cheng@gmail.com> writes:
>> On Wed, Jun 28, 2017 at 2:36 PM, Richard Sandiford
>> <richard.sandiford@linaro.org> wrote:
>>> Index: gcc/tree-data-ref.c
>>> ===================================================================
>>> --- gcc/tree-data-ref.c 2017-06-28 14:33:41.294720044 +0100
>>> +++ gcc/tree-data-ref.c 2017-06-28 14:35:30.475155670 +0100
>>> @@ -749,15 +749,29 @@ canonicalize_base_object_address (tree a
>>>    return build_fold_addr_expr (TREE_OPERAND (addr, 0));
>>>  }
>>>
>>> -/* Analyzes the behavior of the memory reference DR in the innermost loop or
>>> -   basic block that contains it.  Returns true if analysis succeed or false
>>> -   otherwise.  */
>>> +/* Analyze the behavior of memory reference DR.  There are two modes:
>>> +
>>> +   - BB analysis.  In this case we simply split the address into base,
>>> +     init and offset components, without reference to any containing loop.
>>> +     The resulting base and offset are general expressions and they can
>>> +     vary arbitrarily from one iteration of the containing loop to the next.
>>> +     The step is always zero.
>>> +
>>> +   - loop analysis.  In this case we analyze the reference both wrt LOOP
>>> +     and on the basis that the reference occurs (is "used") in LOOP;
>>> +     see the comment above analyze_scalar_evolution_in_loop for more
>>> +     information about this distinction.  The base, init, offset and
>>> +     step fields are all invariant in LOOP.
>>> +
>>> +   Perform BB analysis if LOOP is null, or if LOOP is the function's
>>> +   dummy outermost loop.  In other cases perform loop analysis.
>>> +
>>> +   Return true if the analysis succeeded and store the results in DR if so.
>>> +   BB analysis can only fail for bitfield or reversed-storage accesses.  */
>>>
>>>  bool
>>> -dr_analyze_innermost (struct data_reference *dr, struct loop *nest)
>>> +dr_analyze_innermost (struct data_reference *dr, struct loop *loop)
>>>  {
>>> -  gimple *stmt = DR_STMT (dr);
>>> -  struct loop *loop = loop_containing_stmt (stmt);
>>>    tree ref = DR_REF (dr);
>>>    HOST_WIDE_INT pbitsize, pbitpos;
>>>    tree base, poffset;
>>> @@ -806,22 +820,11 @@ dr_analyze_innermost (struct data_refere
>>>
>>>    if (in_loop)
>> A nit.  No need to use bool variable now?  We can check loop != NULL instead.
>
> As the comment says, we also want to do BB analysis if the loop is the
> outermost dummy loop, so in_loop remains set to:
>
>   bool in_loop = (loop && loop->num);
>
> I think it's worth keeping that abstraction.
Sorry if I misunderstand this.   "loop != NULL" is different to
in_loop only when loop is the outermost dummy loop.  Which means nest
!= NULL based on change:
-  dr_analyze_innermost (dr, nest);
+  dr_analyze_innermost (dr, nest != NULL ? loop : NULL);

I wonder if it's possible for caller to pass non-NULL nest when
create_data_ref for a statement in outermost dummy loop?  Or should
it?

Also noticed that in vect_analyze_data_refs, there is below code:

      /* If target supports vector gather loads or scatter stores, or if
         this might be a SIMD lane access, see if they can't be used.  */
      if (is_a <loop_vec_info> (vinfo)
          && (maybe_gather || maybe_scatter || maybe_simd_lane_access)
          && !nested_in_vect_loop_p (loop, stmt))
        {
          struct data_reference *newdr
        = create_data_ref (NULL, loop_containing_stmt (stmt),
                   DR_REF (dr), stmt, maybe_scatter ? false : true);

we have nest==NULL and loop is the inner loop, but after change NULL
is passed to dr_analyze_innermost because of:

-  dr_analyze_innermost (dr, nest);
+  dr_analyze_innermost (dr, nest != NULL ? loop : NULL);

Is this the reason that bb-slp-pr65935.c gets vectorized after change?
 simple_iv should return {&s->phase[dir], 0} successfully though.
Question is what would happen if simple_iv succeeds with non-ZERO step
when called with nest==NULL?  The patch skips simple_iv and forces
ZERO step?

Thanks,
bin
>
> Thanks,
> Richard
>