Re: [PATCH v3 03/11] target/s390x: vxeh2: vector string search

[David Miller]

I came to much the same conclusion

On Tue, Mar 22, 2022 at 10:42 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 3/21/22 03:31, David Hildenbrand wrote:
> >> +        for (i = 0; i < nelem; i++) {
> >> +            if (s390_vec_read_element(v3, i, es) == 0) {
> >> +                break;
> >> +            }
> >> +        }
> >> +        if (i < substr_elen) {
> >> +            substr_elen = i;
> >> +        }
> >
> > Maybe combine both, I guess there is no need to search beyond substr_elen.
> >
> > substr_elen = MIN(substr_elen, nelem);
> > for (i = 0; i < substr_elen; i++) {
> >      if (s390_vec_read_element(v3, i, es) == 0) {
> >          substr_elen = i;
> >          break;
> >      }
> > }
>
> Yep.
>
> > We should do the MIN(substr_elen, nelem) maybe right when reading it
> > from v4.
>
> No, v4 does not get bounded until zs is set.
>
> >> +    /* If ZS, look for eos in the searched string. */
> >> +    if (zs) {
> >> +        for (k = 0; k < nelem; k++) {
> >> +            if (s390_vec_read_element(v2, k, es) == 0) {
> >> +                eos = true;
> >> +                break;
> >> +            }
> >> +        }
> >
> > I guess we could move that into the main search loop and avoid parsing
> > the string twice. Not sure what's better.
>
> I'd leave it here, so that we only do the strlen once.  There's no obvious 
> place within
> the the search loop that wouldn't wind up doing the strlen more than once.
>
>
> r~