Re: [PATCH 4/5] vect: Request no-wrap bounds from DR analysis.

>> +/* Reset DR_INNERMOST, DR_NOWRAP_BOUND, and access functions of DR.  
>> */
>> +
>> +void
>> +vect_reset_dr (data_reference *dr)

>
> I think this belongs in tree-data-ref.cc, but I also wonder whether
> it should be done upon analysis therein already?

>> +       }
>> +
>> +      if (assum && integer_onep (assum))
>> +       continue;
>> +
>> +      if (!assum || integer_zerop (assum))
>> +       {
>> +         vect_reset_dr (dr);
>
> See above.  Btw, what happens after this?  We'll eventually get
> gather/scatter?

Yes, that reset is to get a more graceful degradation.  I added a 
comment and moved it to tree-data-ref.cc.  At the same time I tried 
defending why we still need a reset here but failed.  I was thinking of 
contradicting bound requirements originally but I can't seem to come up 
with a test.  With the assumption structure being count < bound it's 
difficult (impossible?) to construct such a case.

What I did get to is a test where a "stupid" bound like 1 spoils the 
whole assumption and don't vectorize despite having a valid assumption.
I'll retreat to the argument that a DR that wraps at "1" wouldn't have 
performed well with gather/scatter either and falling back to scalar is 
not too bad.  But of course we could do better there.

>
>> +
>> +         if (dump_enabled_p ())
>> +           dump_printf_loc (MSG_NOTE, vect_location,
>> +                            "data reference address wraps, discarding "
>> +                            "no-wrap assumption: %G", DR_STMT (dr));
>> +         continue;
>> +       }
>> +
>> +      /* Accumulate assumptions.  */
>> +      if (nowrap_assum && nowrap_assum != assum)
>> +       nowrap_assum = fold_build2 (TRUTH_AND_EXPR, boolean_type_node,
>> +                                   nowrap_assum, assum);
>> +      else
>> +       nowrap_assum = assum;
>
> I wonder how ugly "related" assumptions get for grouped accesses?

For example :)

adding no-wrap assumption: ((((unsigned int) ~s1_51(D) >= (unsigned int) 
n_57(D) && (unsigned int) ~s2_53(D) >= (unsigned int) n_57(D)) && 
(unsigned int) ~s2_53(D) >= (unsigned int)
n_57(D)) && (unsigned int) ~s2_53(D) >= (unsigned int) n_57(D)) && 
(unsigned int) ~s2_53(D) >= (unsigned int) n_57(D)

where we don't even eliminate identical terms...

Perhaps we can do better by doing
  IV < MIN (bound1, bound2, ...)
rather than
  IV < bound1 && IV < bound2 ...
Testing that.

>> +      /* We consider to vectorize this loop by versioning it under
>> +        some assumptions.  In order to do this, we need to clear
>> +        existing information computed by scev and niter analyzer.  */
>> +      scev_reset_htab ();
>> +      free_numbers_of_iterations_estimates (loop);
>> +      /* Also set flag for this loop so that following scev and niter
>> +        analysis are done under the assumptions.  */
>> +      loop_constraint_set (loop, LOOP_C_FINITE);
>
> So this is still needed?

Yes for now, but not conceptually.  I have a local patch getting rid of 
it but it's not battle hardened yet.

The rest of your remarks are incorporated and I'm currently re-testing 
the whole batch, hoping to send out the version tomorrow.

-- 
Regards
 Robin