Dmitrij Pochepko <dmitrij.poche...@bell-sw.com> writes: > Hi, > > thank you for reviewing it. > > Please check updated version(attached) with all comments addressed. > > Thanks, > Dmitrij > > On Tue, Jun 23, 2020 at 06:10:52PM +0100, Richard Sandiford wrote: > ... >> >> I think it would be better to test this as part of the loop below. >> > done
For this point, I meant that we should remove the first loop too. I.e.: > + unsigned int encoded_nelts = d->perm.encoding ().encoded_nelts (); > + for (unsigned int i = 0; i < encoded_nelts; ++i) > + if (!d->perm[i].is_constant ()) > + return false; is now redundant with the later: > + for (unsigned HOST_WIDE_INT i = 0; i < nelt; i ++) > + { > + poly_int64 elt = d->perm[i]; > + if (!elt.is_constant ()) > + return false; > + if (elt.to_constant () == (HOST_WIDE_INT) i) > + continue; However, a more canonical way to write the condition above is: for (unsigned HOST_WIDE_INT i = 0; i < nelt; i++) { HOST_WIDE_INT elt; if (!elt.is_constant (&elt)) return false; if (elt == (HOST_WIDE_INT) i) continue; Very minor, but the coding conventions don't put a space before “++”. So: > + for (unsigned HOST_WIDE_INT i = 0; i < nelt; i ++) …this should be “i++” too. Looks good otherwise, thanks. Richard