https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118415
--- Comment #6 from GCC Commits <cvs-commit at gcc dot gnu.org> --- The master branch has been updated by Jeff Law <l...@gcc.gnu.org>: https://gcc.gnu.org/g:9c387a99a911724546abe99ecd39bfc968ed6333 commit r15-6843-g9c387a99a911724546abe99ecd39bfc968ed6333 Author: Jakub Jelinek <ja...@redhat.com> Date: Sun Jan 12 17:24:53 2025 -0700 [PATCH] crc: Fix up some crc related wrong code issues [PR117997, PR118415] Hi! As mentioned in the second PR, using table names like crc_table_for_crc_8_polynomial_0x12 in the user namespace is wrong, user could have defined such variables in their code and as can be seen on the last testcase, then it just misbehaves. At minimum such names should start with 2 underscores, moving it into implementation namespace, and if possible have some dot or dollar in the name if target supports it. I think assemble_crc_table right now always emits tables a local variables, I really don't see what would be setting TREE_PUBLIC flag on IDENTIFIER_NODEs. It might be nice to share the tables between TUs in the same binary or shared library, but it in that case should have hidden visibility if possible, so that it isn't exported from the libraries or binaries, we don't want the optimization to affect set of exported symbols from libraries. And, as can be seen in the first PR, building gen_rtx_SYMBOL_REF by hand is certainly unexpected on some targets, e.g. those which use -fsection-anchors, so we should instead use DECL_RTL of the VAR_DECL. For that we'd need to look it up if we haven't emitted it already, while IDENTIFIER_NODEs can be looked up easily, I guess for the VAR_DECLs we'd need custom hash table. Now, all of the above (except sharing between multiple TUs) is already implemented in output_constant_def, so I think it is much better to just use that function. And, if we want to share it between multiple TUs, we could extend the SHF_MERGE usage in gcc, currently we only use it for constant pool entries with same size as alignment, from 1 to 32 bytes, using .rodata.cstN sections. We could just use say .rodata.cstM.N sections where M would be alignment and N would be the entity size. We could use that for all constant pool entries say up to 2048 bytes. Though, as the current code doesn't share between multiple TUs, I think it can be done incrementally (either still for GCC 15, or GCC 16+). Bootstrapped/regtested on {x86_64,i686,aarch64,powerpc64le,s390x}-linux, on aarch64 it also fixes -FAIL: crypto/rsa -FAIL: hash ok for trunk? gcc/ PR tree-optimization/117997 PR middle-end/118415 * expr.cc (assemble_crc_table): Make static, remove id argument, use output_constant_def. Emit note if -fdump-rtl-expand-details about which table has been emitted. (generate_crc_table): Make static, adjust assemble_crc_table caller, call it always. (calculate_table_based_CRC): Make static. * internal-fn.cc (expand_crc_optab_fn): Emit note if -fdump-rtl-expand-details about using optab for crc. Formatting fix. gcc/testsuite/ * gcc.dg/crc-builtin-target32.c: Add -fdump-rtl-expand-details as dg-additional-options. Scan expand dump rather than assembly, adjust the regexps. * gcc.dg/crc-builtin-target64.c: Likewise. * gcc.dg/crc-builtin-rev-target32.c: Likewise. * gcc.dg/crc-builtin-rev-target64.c: Likewise. * gcc.dg/pr117997.c: New test. * gcc.dg/pr118415.c: New test.
