Hi,

> -----Original Message-----
> From: Richard Sandiford [mailto:richard.sandif...@arm.com]
> Sent: Wednesday, July 1, 2020 9:03 PM
> To: Yangfei (Felix) <felix.y...@huawei.com>
> Cc: Richard Biener <rguent...@suse.de>; Richard Biener
> <richard.guent...@gmail.com>; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH PR95961] vect: ICE: in exact_div, at poly-int.h:2182
> 
> "Yangfei (Felix)" <felix.y...@huawei.com> writes:
> >> On June 30, 2020 4:23:03 PM GMT+02:00, Richard Sandiford
> >> <richard.sandif...@arm.com> wrote:
> >> >Richard Biener <rguent...@suse.de> writes:
> >> >> So it seems odd to somehow put in the number of vectors...  so to
> >> >> me it would have made sense if it did
> >> >>
> >> >>   possible_npeel_number = lower_bound (nscalars);
> >> >>
> >> >> or whateveris necessary to make the polys happy.  Thus simply
> >> >> elide the vect_get_num_vectors call?  But it's been very long
> >> >> since I've dived into the alignment peeling code...
> >> >
> >> >Ah, I see what you mean.  So rather than:
> >> >
> >> >        /* Save info about DR in the hash table.  Also include peeling
> >> >           amounts according to the explanation above.  */
> >> >              for (j = 0; j < possible_npeel_number; j++)
> >> >                {
> >> >                  vect_peeling_hash_insert (&peeling_htab, loop_vinfo,
> >> >                                      dr_info, npeel_tmp);
> >> >            npeel_tmp += target_align / dr_size;
> >> >                }
> >> >
> >> >just have something like:
> >> >
> >> >        while (known_le (npeel_tmp, nscalars))
> >> >          {
> >> >            …
> >> >          }
> >> >
> >> >?
> >>
> >> Yeah.
> >
> > Not sure if I understand correctly.  I am supposing the following check in
> the original code is not necessary if we go like that.
> >
> > 1822               if (unlimited_cost_model (LOOP_VINFO_LOOP (loop_vinfo)))
> >
> > Is that correct?
> 
> I think we still need it.  I guess there are two choices:
> 
> - make nscalars default to npeel_tmp before the “if” above.

I think this will be simpler.  How about the v2 patch?
Bootstrapped and tested on aarch64-linux-gnu & x86_64-linux-gnu.

Thanks,
Felix

Attachment: pr95961-v2.diff
Description: pr95961-v2.diff

Reply via email to