[RFC/RFA][PATCH v2 03/12] RISC-V: Add CRC expander to generate faster CRC.

[Jeff Law]

On 7/26/24 12:06 PM, Mariam Arutunian wrote:
>    If the target is ZBC or ZBKC, it uses clmul instruction for the CRC 
> calculation.
> Otherwise, if the target is ZBKB, generates table-based CRC, but for 
> reversing inputs and the output uses bswap and brev8 instructions.
>    Add new tests to check CRC generation for ZBC, ZBKC and ZBKB targets.
> 
>       gcc/
> 
>          * expr.cc (reflect): New function.
>          (gf2n_poly_long_div_quotient): Likewise.
>          * expr.h (reflect): New function declaration.
>          (gf2n_poly_long_div_quotient): Likewise.
> 
>       gcc/config/riscv/
> 
>          * bitmanip.md (crc_rev<ANYI1:mode><ANYI:mode>4): New expander 
> for reversed CRC.
>          (crc<SUBX1:mode><SUBX:mode>4): New expander for bit-forward CRC.
>          (SUBX1, ANYI1): New iterators.
>          * riscv-protos.h (generate_reflecting_code_using_brev): New 
> function declaration.
>          (expand_crc_using_clmul): Likewise.
>          (expand_reversed_crc_using_clmul): Likewise.
>          * riscv.cc (generate_reflecting_code_using_brev): New function.
>          (expand_crc_using_clmul): Likewise.
>          (expand_reversed_crc_using_clmul): Likewise.
>          * riscv.md (UNSPEC_CRC, UNSPEC_CRC_REV):  New unspecs.
> 
>       gcc/testsuite/gcc.target/riscv/
> 
>             * crc-1-zbc.c: New test.
>             * crc-1-zbc.c: Likewise.
>             * crc-10-zbc.c: Likewise.
>             * crc-10-zbkc.c: Likewise.
>             * crc-12-zbc.c: Likewise.
>             * crc-12-zbkc.c: Likewise.
>             * crc-13-zbc.c: Likewise.
>             * crc-13-zbkc.c: Likewise.
>             * crc-14-zbc.c: Likewise.
>             * crc-14-zbkc.c: Likewise.
>             * crc-17-zbc.c: Likewise.
>             * crc-17-zbkc.c: Likewise.
>             * crc-18-zbc.c: Likewise.
>             * crc-18-zbkc.c: Likewise.
>             * crc-21-zbc.c: Likewise.
>             * crc-21-zbkc.c: Likewise.
>             * crc-22-rv64-zbc.c: Likewise.
>             * crc-22-rv64-zbkb.c: Likewise.
>             * crc-22-rv64-zbkc.c: Likewise.
>             * crc-23-zbc.c: Likewise.
>             * crc-23-zbkc.c: Likewise.
>             * crc-4-zbc.c: Likewise.
>             * crc-4-zbkb.c: Likewise.
>             * crc-4-zbkc.c: Likewise.
>             * crc-5-zbc.c: Likewise.
>             * crc-5-zbkb.c: Likewise.
>             * crc-5-zbkc.c: Likewise.
>             * crc-6-zbc.c: Likewise.
>             * crc-6-zbkc.c: Likewise.
>             * crc-7-zbc.c: Likewise.
>             * crc-7-zbkc.c: Likewise.
>             * crc-8-zbc.c: Likewise.
>             * crc-8-zbkb.c: Likewise.
>             * crc-8-zbkc.c: Likewise.
>             * crc-9-zbc.c: Likewise.
>             * crc-9-zbkc.c: Likewise.
>             * crc-CCIT-data16-zbc.c: Likewise.
>             * crc-CCIT-data16-zbkc.c: Likewise.
>             * crc-CCIT-data8-zbc.c: Likewise.
>             * crc-CCIT-data8-zbkc.c: Likewise.
>             * crc-coremark-16bitdata-zbc.c: Likewise.
>             * crc-coremark-16bitdata-zbkc.c: Likewise.
> 
>     Signed-off-by: Mariam Arutunian <mariamarutunian@gmail.com 
> <mailto:mariamarutunian@gmail.com>>
> 
> 
> 0003-RISC-V-Add-CRC-expander-to-generate-faster-CRC.patch
> 
> diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md
> index 8769a6b818b..9683ac48ef6 100644
> --- a/gcc/config/riscv/bitmanip.md
> +++ b/gcc/config/riscv/bitmanip.md
> @@ -973,3 +973,67 @@
>     "TARGET_ZBC"
>     "clmulr\t%0,%1,%2"
>     [(set_attr "type" "clmul")])
> +
> +
> +;; Iterator for hardware integer modes narrower than XLEN, same as SUBX
> +(define_mode_iterator SUBX1 [QI HI (SI "TARGET_64BIT")])
> +
> +;; Iterator for hardware integer modes narrower than XLEN, same as ANYI
> +(define_mode_iterator ANYI1 [QI HI SI (DI "TARGET_64BIT")])
Might as well go ahead and put these into iterators.md.


> +
> +;; Reversed CRC 8, 16, 32 for TARGET_64
> +(define_expand "crc_rev<ANYI1:mode><ANYI:mode>4"
> +	;; return value (calculated CRC)
> +  [(set (match_operand:ANYI 0 "register_operand" "=r")
> +		      ;; initial CRC
> +	(unspec:ANYI [(match_operand:ANYI 1 "register_operand" "r")
> +		      ;; data
> +		      (match_operand:ANYI1 2 "register_operand" "r")
> +		      ;; polynomial without leading 1
> +		      (match_operand:ANYI 3)]
> +		      UNSPEC_CRC_REV))]
> +  /* We don't support the case when data's size is bigger than CRC's size.  */
> +  "(((TARGET_ZBKC || TARGET_ZBC) && <ANYI:MODE>mode < word_mode)
> +    || TARGET_ZBKB) && <ANYI:MODE>mode >= <ANYI1:MODE>mode"
This condition should get reformatted.   Ideally the condition should be 
fairly obvious, but it's fairly obfuscated here.

I would expect that the TARGET_ZBKB likely belongs inside the 
conditional with the other TARGET tests.  Perhaps something like this:

> "(TARGET_ZBKB || TARGET_ZBKC || TARGET_ZBC)
>  && <ANYI:MODE>mode < word_mode
>  && <ANYI:MODE>mode >= <ANYI1:MODE>mode"

Or did you mean to allow ZBKB even if the quotient needs 65 bits?  If so 
the condition still needs adjustment, just a different adjustment.


Otherwise this looks fine to me.  Were there any adjustments you were 
considering after working through the aarch64 expansion with Richard S?

jeff