On Tue, 2026-03-31 at 07:56 -0600, Jeffrey Law wrote: > > > On 11/28/2025 2:58 AM, Kito Cheng wrote: > > I know we're already in stage 3, so I'm basically asking for a review and am > > fine with deferring this to GCC 17. But if it's acceptable for GCC 16, that > > would be great too. :P > > > > --- > > > > The previous reversed CRC implementation used explicit bit reflection > > before and after the CRC computation: > > > > reflect(crc_init); > > reflect(data); > > for (int i = 0; i < data_bit_size / 8; i++) > > crc = (crc << 8) ^ table[(crc >> (crc_bit_size - 8)) > > ^ (data >> (data_bit_size - (i+1) * 8) & > > 0xFF)]; > > reflect(crc); > > > > This patch generates a reversed polynomial lookup table directly, > > eliminating the need for bit reflection operations. The new algorithm: > > > > for (int i = 0; i < data_bit_size / 8; i++) > > crc = (crc >> 8) ^ table[(crc ^ (data >> (i * 8))) & 0xFF]; > > > > This improves code generation for all targets using table-based reversed > > CRC, as it removes the overhead of reflecting input data and CRC values. > > > > Ref: > > > > [1] "Reversing CRC - Theory and Practice" > > > > https://sar.informatik.hu-berlin.de/research/publications/SAR-PR-2006-05/SAR-PR-2006-05_.pdf > > > > gcc/ChangeLog: > > > > * expr.cc (calculate_reversed_crc): New function. > > (assemble_reversed_crc_table): New function. > > (generate_reversed_crc_table): New function. > > (calculate_table_based_reversed_CRC): New function. > > (expand_reversed_crc_table_based): Remove gen_reflecting_code > > parameter. Use calculate_table_based_reversed_CRC. > > * expr.h (expand_reversed_crc_table_based): Update prototype. > > * builtins.cc (expand_builtin_crc_table_based): Update call. > > * internal-fn.cc (expand_crc_optab_fn): Update call. > > * config/aarch64/aarch64.md (crc_rev<ALLI:mode><ALLX:mode>4): > > Update call. > > * config/i386/i386.md (crc_rev<SWI124:mode>si4): Update call. > > * config/loongarch/loongarch.md (crc_rev<mode>si4): Update call. > > Remove local rbit lambda. > > * config/riscv/bitmanip.md (crc_rev<ANYI1:mode><ANYI:mode>4): > > Update call. Remove TARGET_ZBKB case. > > * config/riscv/riscv.cc (generate_reflecting_code_using_brev): > > Remove. > > * config/riscv/riscv-protos.h (generate_reflecting_code_using_brev): > > Remove declaration. > > --- > > gcc/builtins.cc | 3 +- > > gcc/config/aarch64/aarch64.md | 3 +- > > gcc/config/i386/i386.md | 3 +- > > gcc/config/loongarch/loongarch.md | 17 +--- > > gcc/config/riscv/bitmanip.md | 13 +-- > > gcc/config/riscv/riscv-protos.h | 1 - > > gcc/config/riscv/riscv.cc | 33 ------ > > gcc/expr.cc | 160 ++++++++++++++++++++++++++---- > > gcc/expr.h | 3 +- > > gcc/internal-fn.cc | 3 +- > > 10 files changed, 148 insertions(+), 91 deletions(-) > > > > > > > > > > > > +/* Generate table-based reversed CRC code for the given CRC, INPUT_DATA > > + and the POLYNOMIAL (without leading 1). > > + > > + This function generates code for reversed (bit-reflected) CRC > > calculation > > + using a pre-computed lookup table. Unlike the standard CRC calculation, > > + this processes data from LSB to MSB, eliminating the need for explicit > > + bit reflection before and after the CRC computation. */ > > + > > +static void > > +calculate_table_based_reversed_CRC (rtx *crc, const rtx &input_data, > > + const rtx &polynomial, > > + machine_mode data_mode) > So in the main loop of this function (elided for brevity) you convert > INPUT_DATA to another type and store the result in DATA. I believe you > can do the conversion outside the loop since INPUT_DATA is unchanged. > It probably doesn't matter in practice though, so consider it an > implementation nit. > > Otherwise it looks like a really nice cleanup to me. OK for gcc-17 > once it's open for development, with or without the nit above addressed.
Gentle ping: I have some changes to adapt LoongArch for r17-523 and if this patch is pushed first I'll not need to care about the nasty lambda handling bit reversing in LoongArch crc_rev<mode>si4. -- Xi Ruoyao <[email protected]>
