On Tue, Nov 12, 2024 at 12:31 AM Jeff Law <jeffreya...@gmail.com> wrote:
>
>
> On 11/9/24 12:43 PM, Mariam Arutunian wrote:
> > Add two new internal functions (IFN_CRC, IFN_CRC_REV), to provide faster
> > CRC generation.
> > One performs bit-forward and the other bit-reversed CRC computation.
> > If CRC optabs are supported, they are used for the CRC computation.
> > Otherwise, table-based CRC is generated.
> > The supported data and CRC sizes are 8, 16, 32, and 64 bits.
> > The polynomial is without the leading 1.
> > A table with 256 elements is used to store precomputed CRCs.
> > For the reflection of inputs and the output, a simple algorithm involving
> > SHIFT, AND, and OR operations is used.
> >
> > gcc/
> >
> > * doc/md.texi (crc@var{m}@var{n}4,
> > crc_rev@var{m}@var{n}4): Document.
> > * expr.cc (calculate_crc): New function.
> > (assemble_crc_table): Likewise.
> > (generate_crc_table): Likewise.
> > (calculate_table_based_CRC): Likewise.
> > (emit_crc): Likewise.
> > (expand_crc_table_based): Likewise.
> > (gen_common_operation_to_reflect): Likewise.
> > (reflect_64_bit_value): Likewise.
> > (reflect_32_bit_value): Likewise.
> > (reflect_16_bit_value): Likewise.
> > (reflect_8_bit_value): Likewise.
> > (generate_reflecting_code_standard): Likewise.
> > (expand_reversed_crc_table_based): Likewise.
> > * expr.h (generate_reflecting_code_standard): New function
> declaration.
> > (expand_crc_table_based): Likewise.
> > (expand_reversed_crc_table_based): Likewise.
> > * internal-fn.cc: (crc_direct): Define.
> > (direct_crc_optab_supported_p): Likewise.
> > (expand_crc_optab_fn): New function
> > * internal-fn.def (CRC, CRC_REV): New internal functions.
> > * optabs.def (crc_optab, crc_rev_optab): New optabs.
> >
> > Signed-off-by: Mariam Arutunian <mariamarutun...@gmail.com
> > <mailto:mariamarutun...@gmail.com>>
> > Co-authored-by: Joern Rennecke <joern.renne...@embecosm.com
> > <mailto:joern.renne...@embecosm.com>>
> > Mentored-by: Jeff Law <j...@ventanamicro.com <mailto:
> j...@ventanamicro.com>>
> >
> > 0001-Implement-internal-functions-for-efficient-CRC-compu.patch
> >
> > ---
> > gcc/doc/md.texi | 14 ++
> > gcc/expr.cc | 372 ++++++++++++++++++++++++++++++++++++++++++++
> > gcc/expr.h | 6 +
> > gcc/internal-fn.cc | 54 +++++++
> > gcc/internal-fn.def | 2 +
> > gcc/optabs.def | 2 +
> > 6 files changed, 450 insertions(+)
> >
> > diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
> > index a9259112251..913d1f96373 100644
> > --- a/gcc/doc/md.texi
> > +++ b/gcc/doc/md.texi
> > @@ -8591,6 +8591,20 @@ Return 1 if operand 1 is a normal floating point
> number and 0
> > otherwise. @var{m} is a scalar floating point mode. Operand 0
> > has mode @code{SImode}, and operand 1 has mode @var{m}.
> >
> > +@cindex @code{crc@var{m}@var{n}4} instruction pattern
> > +@item @samp{crc@var{m}@var{n}4}
> > +Calculate a bit-forward CRC using operands 1, 2 and 3,
> > +then store the result in operand 0.
> > +Operands 1 is the initial CRC, operands 2 is the data and operands 3 is
> the
> > +polynomial without leading 1.
> > +Operands 0, 1 and 3 have mode @var{n} and operand 2 has mode @var{m},
> where
> > +both modes are integers. The size of CRC to be calculated is
> determined by the
> > +mode; for example, if @var{n} is 'hi', a CRC16 is calculated.
> Nit. When we reference modes in documentation we typically do so with
> something like this @code{HImode}.
>
>
>
> > +
> > +/* Converts and moves a CRC value to a target register.
> > +
> > + CRC_MODE is the mode (data type) of the CRC value.
> > + CRC is the initial CRC value.
> > + OP0 is the target register. */
> > +
> > +void
> > +emit_crc (machine_mode crc_mode, rtx* crc, rtx* op0)
> > +{
> > + if (word_mode != crc_mode)
> > + {
> > + rtx tgt = simplify_gen_subreg (word_mode, *op0, crc_mode, 0);
> Can CRC_MODE ever be wider than WORD_MODE?
>
> If so, then that last argument needs adjustment to cope with endianness.
> On the optimistic assumption that we're always generating a
> paradoxical, I'm putting a gcc_assert in my local copy for testing
> purposes.
>
>
>
>
> > + rtx crc_low = gen_lowpart (crc_mode, *crc);
> > + if (SUBREG_P (*op0) && SUBREG_PROMOTED_VAR_P (*op0))
> > + convert_move (tgt, crc_low, SUBREG_PROMOTED_SIGN (*op0));
> > + else
> > + convert_move (tgt, crc_low, 0);
> > + }
> > + else
> > + emit_move_insn (*op0, *crc);
> Formatting nit. The final "else" and "emit_move_insn" call are
> mis-indented. I've fix that in my local copy.
>
>
>
> > +
> > +/* Reflect 64-bit value for the 64-bit target. */
> > +
> > +void
> > +reflect_64_bit_value (rtx *op)
> > +{
> > + gen_common_operation_to_reflect (op, 0x00000000FFFFFFFF,
> > + 0xFFFFFFFF00000000, 32);
> > + gen_common_operation_to_reflect (op, 0x0000FFFF0000FFFF,
> > + 0xFFFF0000FFFF0000, 16);
> > + gen_common_operation_to_reflect (op, 0x00FF00FF00FF00FF,
> > + 0xFF00FF00FF00FF00, 8);
> > + gen_common_operation_to_reflect (op, 0x0F0F0F0F0F0F0F0F,
> > + 0xF0F0F0F0F0F0F0F0, 4);
> > + gen_common_operation_to_reflect (op, 0x3333333333333333,
> > + 0xCCCCCCCCCCCCCCCC, 2);
> > + gen_common_operation_to_reflect (op, 0x5555555555555555,
> > + 0xAAAAAAAAAAAAAAAA, 1);
> > +}
> I should have caught this before. These constants probably need to be
> wrapped by a HOST_WIDE_INT_C. I'll fix that locally too. I don't think
> the other routines for 32, 16 or 8 bit need this.
>
>
Thank you very much.
Since you have already fixed the mentioned issues locally, I think there is
no need for me to send a revised version.
Thanks,
Mariam
> jeff
>
