Re: [PATCH 7/7] riscv: Add support for str(n)cmp inline expansion

[Jeff Law <jeffreyalaw@gmail.com>]

On 11/14/22 17:53, Palmer Dabbelt wrote:
> On Mon, 14 Nov 2022 16:46:37 PST (-0800), Kito Cheng wrote:
>> Hi Christoph:
>>
>>> This patch implements expansions for the cmpstrsi and the cmpstrnsi
>>> builtins using Zbb instructions (if available).
>>> This allows to inline calls to strcmp() and strncmp().
>>>
>>> The expansion basically emits a peeled comparison sequence (i.e. a 
>>> peeled
>>> comparison loop) which compares XLEN bits per step if possible.
>>>
>>> The emitted sequence can be controlled, by setting the maximum number
>>> of compared bytes (-mstring-compare-inline-limit).
>>
>> I would like to have a unified option interface,
>> maybe -m[no-]inline-str[n]cmp and -minline-str[n]cmp-limit.
>> And add some option like this:
>> -minline-str[n]cmp=[bitmanip|vector|auto] in future,
>> since I assume we'll have different versions of those things.
>
> Can we just decide that from mtune?  We'll probably have 
> uarch-specific string functions at some point, might as well start 
> planning for it now.

Sure, though the implementation isn't terribly tied to any uarch at the 
moment and I doubt uarch approaches would make significant impacts here 
-- we're peeling off some small number of iterations fairly 
generically.  The uarch specific stuff would be the code in glibc 
selected by an ifunc.  uarch variants for block copiers seem inevitable 
though :-)


I don' t have strong opinions here, so if we want to key off mtune, 
sure.  If we want to have variants for scalar vs vector that's quite 
reasonable too.  Or if we want to go all the way to uarch specific 
implementations, I won't object.






> th
>>> gcc/ChangeLog:
>>>
>>>         * config/riscv/riscv-protos.h (riscv_expand_strn_compare): New
>>>           prototype.
>>>         * config/riscv/riscv-string.cc (GEN_EMIT_HELPER3): New helper
>>>           macros.
>>>         (GEN_EMIT_HELPER2): New helper macros.
>>>         (expand_strncmp_zbb_sequence): New function.
>>>         (riscv_emit_str_compare_zbb): New function.
>>>         (riscv_expand_strn_compare): New function.
>>>         * config/riscv/riscv.md (cmpstrnsi): Invoke expansion functions
>>>           for strn_compare.
>>>         (cmpstrsi): Invoke expansion functions for strn_compare.
>>>         * config/riscv/riscv.opt: Add new parameter
>>>           '-mstring-compare-inline-limit'.
>>
>> We need to document this option.

Yes, definitely needs documentation.  Thanks for catching that.


jeff