On 9/13/24 5:06 AM, Mariam Arutunian wrote:
Symbolically execute potential CRC loops and check whether the loop
actually calculates CRC (uses LFSR matching).
Calculated CRC and created LFSR are compared on each iteration of the
potential CRC loop.
gcc/
* Makefile.in (OBJS): Add crc-verification.o.
* crc-verification.cc: New file.
* crc-verification.h: New file.
* gimple-crc-optimization.cc (loop_calculates_crc): New function.
(is_output_crc): Likewise.
(swap_crc_and_data_if_needed): Likewise.
(validate_crc_and_data): Likewise.
(optimize_crc_loop): Likewise.
(table_based_may_be_generated): Likewise.
(get_output_phi): Likewise.
(execute): Add check whether potential CRC loop calculates CRC.
gcc/sym-exec/
* sym-exec-state.cc (create_reversed_lfsr): New function.
(create_forward_lfsr): Likewise.
(last_set_bit): Likewise.
(create_lfsr): Likewise.
* sym-exec-state.h (is_bit_vector): Reorder, make the function
public and static.
(create_reversed_lfsr): New static function declaration.
(create_forward_lfsr): New static function declaration.
Signed-off-by: Mariam Arutunian <mariamarutun...@gmail.com
<mailto:mariamarutun...@gmail.com>>
Mentored-by: Jeff Law <j...@ventanamicro.com <mailto:j...@ventanamicro.com>>
There's a bit of TARGET_XXX stuff in here too that needs to be removed
as I noted in the review of a prior patch in this series:
+/* Returns true if table-based CRC may be generated,
+ otherwise, return false. */
+
+bool
+crc_optimization::table_based_may_be_generated ()
+{
+ #ifdef TARGET_CRC32
+ if (TARGET_CRC32
+ && (m_polynomial == 0x1EDC6F41 || m_polynomial == 0x04C11DB7))
+ return false;
+ #endif
+ /* If CLMUL-based CRC can be generated, return false. */
+ if (is_CLMUL_supported ()
+ && (GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (m_crc_arg))).to_constant ()
+ < GET_MODE_SIZE (word_mode)))
+ return false;
+ return true;
+}
Removing this probably will result in minor simplifications elsewhere in
this file.
This is a bit puzzling:
+bool
+assign_calc_vals_to_header_phis (const vec<state *> &prev_states,
+ state *curr_state,
+ basic_block bb)
+{
+ for (gphi_iterator gsi = gsi_start_phis (bb); !gsi_end_p (gsi);
+ gsi_next (&gsi))
+ {
+ gphi *phi = gsi.phi ();
+ tree lhs = gimple_phi_result (phi);
+
+ /* Don't consider virtual operands. */
+ if (virtual_operand_p (lhs))
+ continue;
+
+ if (TREE_CODE (PHI_ARG_DEF (phi, phi->nargs - 1)) == INTEGER_CST)
The order of elements in the PHI is undefined, it looks like this
assumes the constant is always last. So it probably needs to be
generalized a bit.
Similarly in assign_known_vals_to_header_phis.
Mostly looks OK. I suspect with the issues noted above fixed that this
will be good to go.
Jeff
