Re: [PATCH net-next 1/4] unroll: add generic loop unroll helpers

[Maciej Fijalkowski]

On Sun, Feb 09, 2025 at 11:07:25AM +0000, Simon Horman wrote:
> On Thu, Feb 06, 2025 at 07:26:26PM +0100, Alexander Lobakin wrote:
> > There are cases when we need to explicitly unroll loops. For example,
> > cache operations, filling DMA descriptors on very high speeds etc.
> > Add compiler-specific attribute macros to give the compiler a hint
> > that we'd like to unroll a loop.
> > Example usage:
> > 
> >  #define UNROLL_BATCH 8
> > 
> > 	unrolled_count(UNROLL_BATCH)
> > 	for (u32 i = 0; i < UNROLL_BATCH; i++)
> > 		op(priv, i);
> > 
> > Note that sometimes the compilers won't unroll loops if they think this
> > would have worse optimization and perf than without unrolling, and that
> > unroll attributes are available only starting GCC 8. For older compiler
> > versions, no hints/attributes will be applied.
> > For better unrolling/parallelization, don't have any variables that
> > interfere between iterations except for the iterator itself.
> > 
> > Co-developed-by: Jose E. Marchesi <jose.marchesi@xxxxxxxxxx> # pragmas
> > Signed-off-by: Jose E. Marchesi <jose.marchesi@xxxxxxxxxx>
> > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@xxxxxxxxx>
> > Signed-off-by: Alexander Lobakin <aleksander.lobakin@xxxxxxxxx>
> 
> Hi Alexander,
> 
> This patch adds four variants of the unrolled helper.  But as far as I can
> tell the patch-set only makes use of one of them, unrolled_count().
> 
> I think it would be best if this patch only added helpers that are used.

That is debatable but I think I tend to agree here. If we add say 3 unused
macros then nothing stops someone from coming up with a patch that deletes
them because they are unused, right?

> 
> ...