On Sun, Aug 25, 2024 at 9:41 PM Jeff Law <jeffreya...@gmail.com> wrote:
> > > 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 <mariamarutun...@gmail.com > > <mailto:mariamarutun...@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. Ok. > > > > + > > +;; 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. Yes, in the case of ZBKB I generate table-based CRC, just use specific instructions to reverse values. Would it be okay if I just leave "<ANYI:MODE>mode >= <ANYI1:MODE>", and then generate a standard table-based implementation in the body if none of the conditions are met? Or just write this way: "(TARGET_ZBKB || ((TARGET_ZBKC || TARGET_ZBC) && <ANYI:MODE>mode < word_mode)) && <ANYI:MODE>mode >= <ANYI1:MODE>mode" > > Otherwise this looks fine to me. Were there any adjustments you were > considering after working through the aarch64 expansion with Richard S? > Yes, I applied some of the changes I made in AArch64 here as well, where possible and worked. Also, in AArch64, I used force_reg in some cases, but here, I didn't change, as you had told me to use riscv_expand_* instead of force_reg. Thanks, Mariam > jeff > > >
