On 5/28/24 1:01 AM, Richard Biener wrote:
On Fri, May 24, 2024 at 10:46 AM Mariam Arutunian
<mariamarutun...@gmail.com> wrote:

This patch adds a new compiler pass aimed at identifying naive CRC 
implementations,
characterized by the presence of a loop calculating a CRC (polynomial long 
division).
Upon detection of a potential CRC, the pass prints an informational message.

Performs CRC optimization if optimization level is >= 2,
besides optimizations for size and if fno_gimple_crc_optimization given.

This pass is added for the detection and optimization of naive CRC 
implementations,
improving the efficiency of CRC-related computations.

This patch includes only initial fast checks for filtering out non-CRCs,
detected possible CRCs verification and optimization parts will be provided in 
subsequent patches.

Just a few quick questions - I'm waiting for a revision with Jeffs comments cleared before having a closer look. The patch does
nothing but analyze right now, correct?  I assume a later patch will
fill in stuff in ::execute and use the return value of
loop_may_calculate_crc (it's a bit odd to review such a "split"
thing).
We split it up on functional chunks. I think if it gets approved it probably should go in atomically since it makes no sense to commit the first pass recognition filter without the validation step or the validation step without the codegen step.

So consider the break down strictly for review convenience.



I think what this does fits final value replacement which lives in tree-scalar-evolution.cc and works from the loop-closed PHIs, trying
to replace those.  I'm not sure we want to have a separate pass for
this. Consider a loop calculating two or four CRCs in parallel, replacing LC PHIs one-by-one should be able to handle this.
I suspect that'll be quite hard for both the "does this generally look like a CRC loop" code as well as the "validate this is a CRC loop" code.

Mariam, your thoughts on whether or not those two phases could handle a loop with two CRC calculations inside, essentially creating two calls to our new builtins?

Jeff


Reply via email to