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

[Richard Henderson]

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~