On 10/8/24 4:52 AM, Mariam Arutunian wrote:
On Sun, Sep 29, 2024 at 9:08 PM Jeff Law <jeffreya...@gmail.com
<mailto:jeffreya...@gmail.com>> wrote:
On 9/13/24 5:05 AM, 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.
Looks pretty good. Just one question/comment:
>
> +void
> +emit_crc (machine_mode crc_mode, rtx* crc, rtx* op0)
> +{
> + if (GET_MODE_BITSIZE (crc_mode).to_constant () == 32
> + && GET_MODE_BITSIZE (word_mode) == 64)
> + {
> + rtx a_low = gen_lowpart (crc_mode, *crc);
> + *crc = gen_rtx_SIGN_EXTEND (word_mode, a_low);
> + }
> + rtx tgt = *op0;
> + if (word_mode != crc_mode)
> + tgt = simplify_gen_subreg (word_mode, *op0, crc_mode, 0);
> + emit_move_insn (tgt, *crc);
> +}
It seems to me that first clause ought to apply any time word mode is
larger than crc mode rather than making it check 32/64 magic constants.
When I apply it whenever the word mode is larger than crc mode, on RISC-
V the CRC-16 and CRC-8 tests fail.
We should work to understand that. Magic constants are generally to be
avoided. There's nothing inherent in this code where those constant
size values should be used.
This is likely pointing to a problem either in how emit_crc is handling
those other cases or in the RISC-V expansion code.
Jeff
