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

[Simon Horman <horms@kernel.org>]

On Mon, Feb 10, 2025 at 04:49:14PM +0100, Alexander Lobakin wrote:
> From: Simon Horman <horms@kernel.org>
> Date: Sun, 9 Feb 2025 11:07:25 +0000
> 
> > 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@oracle.com> # pragmas
> >> Signed-off-by: Jose E. Marchesi <jose.marchesi@oracle.com>
> >> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> >> Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com>
> > 
> > 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.
> 
> I thought they might help people in future.
> I can remove them if you insist. BTW the original patch from Jose also
> added several variants.

I do slightly prefer only adding what is used.