Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
From:   Jani Nikula <jani.nikula@linux.intel.com>
To:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
        linux-kernel@vger.kernel.org
Cc:     Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@intel.com>,
        Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        Geert Uytterhoeven <geert@linux-m68k.org>
Subject: Re: [PATCH v1 1/1] drm/i915/selftests: Replace too verbose for-loop
 with simpler while
In-Reply-To: <20220215163213.54917-1-andriy.shevchenko@linux.intel.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
References: <20220215163213.54917-1-andriy.shevchenko@linux.intel.com>
Date:   Tue, 15 Feb 2022 19:14:49 +0200
Message-ID: <87o83859x2.fsf@intel.com>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

On Tue, 15 Feb 2022, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> It's hard to parse for-loop which has some magic calculations inside.
> Much cleaner to use while-loop directly.

I assume you're trying to prove a point following our recent
for-vs-while loop discussion. I really can't think of any other reason
you'd end up looking at this file or this loop.

With the change, the loop indeed becomes simpler, but it also runs one
iteration further than the original. Whoops.

It's a selftest. The loop's been there for five years. What are we
trying to achieve here? So we disagree on loops, fine. Perhaps this is
not the best use of either of our time? Please just let the for loops in
i915 be.


BR,
Jani.

>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/selftests/i915_syncmap.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/selftests/i915_syncmap.c b/drivers/gpu/drm/i915/selftests/i915_syncmap.c
> index 47f4ae18a1ef..26981d1c3025 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_syncmap.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_syncmap.c
> @@ -36,10 +36,10 @@ __sync_print(struct i915_syncmap *p,
>  	unsigned int i, X;
>  
>  	if (depth) {
> -		unsigned int d;
> +		unsigned int d = depth;
>  
> -		for (d = 0; d < depth - 1; d++) {
> -			if (last & BIT(depth - d - 1))
> +		while (d--) {
> +			if (last & BIT(d))
>  				len = scnprintf(buf, *sz, "|   ");
>  			else
>  				len = scnprintf(buf, *sz, "    ");

-- 
Jani Nikula, Intel Open Source Graphics Center