On Sat, May 25, 2024 at 10:32 PM Jeff Law <jeffreya...@gmail.com> wrote:

>
>
> On 5/24/24 2:41 AM, 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 (gf2n_poly_long_div_quotient): New function.
> >       (reflect): Likewise.
> >       * expr.h (gf2n_poly_long_div_quotient): New function declaration.
> >       (reflect): 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-10-zbc.c: Likewise.
> >          * crc-12-zbc.c: Likewise.
> >          * crc-13-zbc.c: Likewise.
> >          * crc-14-zbc.c: Likewise.
> >          * crc-17-zbc.c: Likewise.
> >          * crc-18-zbc.c: Likewise.
> >          * crc-21-zbc.c: Likewise.
> >          * crc-22-rv64-zbc.c: Likewise.
> >          * crc-22-zbkb.c: Likewise.
> >          * crc-23-zbc.c: Likewise.
> >          * crc-4-zbc.c: Likewise.
> >          * crc-5-zbc.c: Likewise.
> >          * crc-5-zbkb.c: Likewise.
> >          * crc-6-zbc.c: Likewise.
> >          * crc-7-zbc.c: Likewise.
> >          * crc-8-zbc.c: Likewise.
> >          * crc-8-zbkb.c: Likewise.
> >          * crc-9-zbc.c: Likewise.
> >          * crc-CCIT-data16-zbc.c: Likewise.
> >          * crc-CCIT-data8-zbc.c: Likewise.
> >          * crc-coremark-16bitdata-zbc.c: Likewise.
> >
> > Signed-off-by: Mariam Arutunian <mariamarutun...@gmail.com
> > <mailto:mariamarutun...@gmail.com>>
> > diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md
> > index 8769a6b818b..c98d451f404 100644
> > --- a/gcc/config/riscv/bitmanip.md
> > +++ b/gcc/config/riscv/bitmanip.md
> > @@ -973,3 +973,66 @@
> >    "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")])
> If these iterators are the same as existing ones, let's just using the
> existing ones. unless we need both SUBX and SUBX1 in the same pattern or
> ANYI/ANYI1.
>

I need both, as I need to get all the combinations.
For example crc_revsihi4, crc_revsiqi4, etc.
If I write "crc_rev<*ANYI*:mode><ANYI:mode>4" instead of  "crc_rev<*ANYI1*
:mode><ANYI:mode>4",
it will generate only crc_revsisi4, crc_revqiqi4, etc.



> > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > index 85df5b7ab49..123695033a6 100644
> > --- a/gcc/config/riscv/riscv.cc
> > +++ b/gcc/config/riscv/riscv.cc
> > @@ -11394,7 +11394,7 @@ riscv_expand_usadd (rtx dest, rtx x, rtx y)
> >    if (mode == HImode || mode == QImode)
> >      {
> >        int shift_bits = GET_MODE_BITSIZE (Xmode)
> > -     - GET_MODE_BITSIZE (mode).to_constant ();
> > +                    - GET_MODE_BITSIZE (mode).to_constant ();
> >
> >        gcc_assert (shift_bits > 0);
> Looks like an unrelated spurious change.  Drop.
>
>
>

Oh! Thanks for catching that.


> +  riscv_expand_op (LSHIFTRT, word_mode, a0, a0, gen_int_mode (crc_size,
> > +                                                           word_mode));
> Formatting nit on word_size argument.
>
>
> > +
> > +  if (crc_size == 32 && word_mode == DImode)
> Rather than hard coding these cases, would it be better to check if
> crc_size < GET_MODE_BITSIZE (word_mode)?  Similarly for the reversed
> case.

Overall it looks pretty good as well.  Please repost after fixing the
> minor issues noted above.
>
> Thanks,
> jeff
>


The misalignment was due to incorrect settings on my side, which I have now
corrected.
I will apply all the suggestions and repost. Thank you.

Best regards,
Mariam

Reply via email to