On Thu, Aug 22, 2024 at 1:19 PM Richard Biener <richard.guent...@gmail.com>
wrote:
> On Fri, Aug 2, 2024 at 6:15 PM Mariam Arutunian
> <mariamarutun...@gmail.com> wrote:
> >
> > After the loop exit an internal function call (CRC, CRC_REV) is added,
> > and its result is assigned to the output CRC variable (the variable
> where the calculated CRC is stored after the loop execution).
> > The removal of the loop is left to CFG cleanup and DCE.
> >
> > gcc/
> >
> > * gimple-crc-optimization.cc (get_data): New function.
> > (optimize_crc_loop): Likewise.
> > (build_polynomial_without_1): Likewise.
> > (execute): Add optimize_crc_loop function call.
>
> + /* If we have the data, use it. */
> + if (m_phi_for_data)
> + {
> + if (dump_file && (dump_flags & TDF_DETAILS))
> + fprintf (dump_file,
> + "Data and CRC are xor-ed in the for loop. Initializing
> data "
> + "with its value.\n");
> + tree data_arg = PHI_ARG_DEF (m_phi_for_data, 1);
>
> how do you know statically that it's the second PHI arg def? That
> seems fragile.
> I suppose you are looking for a latch definition?
>
I need the one that enters the loop beginning.
Changed to PHI_ARG_DEF_FROM_EDGE.
> + if (TYPE_PRECISION (TREE_TYPE (data_arg)) == data_size)
> + return data_arg;
> + else
> + {
> + if (dump_file && (dump_flags & TDF_DETAILS))
> + fprintf (dump_file,
> + "Loop iteration number and data's size differ.\n");
> + return nullptr;
>
> that seems to be oddly placed in the transform rather than the analysis
> phase?
>
Moved before the verification step.
>
> + gcc_assert (m_phi_for_crc);
> +
> + tree crc_arg = PHI_ARG_DEF (m_phi_for_crc, 1);
>
> same issue here - I think you want to use PHI_ARG_DEF_FROM_EDGE?
>
Changed.
>
> + /* We don't support the case where data is larger than the CRC. */
> + if (TYPE_PRECISION (TREE_TYPE (crc_arg))
> + < TYPE_PRECISION (TREE_TYPE (data_arg)))
> + return false;
>
> you can check that before building any stuff?
>
Yes. I moved, before the verification step.
>
> + gimple_stmt_iterator si = gsi_start_bb (output_crc->bb);
> + gsi_insert_before (&si, call, GSI_SAME_STMT);
>
> you want gsi_after_labels (gimple_bb (output_crc))
>
> + /* Remove phi statement, which was holding CRC result. */
> + gimple_stmt_iterator tmp_gsi = gsi_for_stmt (output_crc);
> + gsi_remove (&tmp_gsi, true);
>
> Please use remove_phi_node (&tmp_gsi, false).
>
>
Done.
> otherwise looks fine.
>
>
Thanks,
Mariam
> Richard.
>
> > Signed-off-by: Mariam Arutunian <mariamarutun...@gmail.com>
>
