LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Lucas De Marchi <lucas.demarchi@intel.com>
To: Steven Price <steven.price@arm.com>
Cc: intel-gfx@lists.freedesktop.org,
	Daniel Vetter <daniel.vetter@intel.com>,
	dri-devel@lists.freedesktop.org,
	Masahiro Yamada <masahiroy@kernel.org>,
	linux-kernel@vger.kernel.org,
	Jani Nikula <jani.nikula@linux.intel.com>
Subject: Re: [PATCH v2 2/3] drm/i915/utils: do not depend on config being defined
Date: Thu, 30 Sep 2021 08:43:15 -0700	[thread overview]
Message-ID: <20210930154315.xb43gowfhmxucsm4@ldmartin-desk2> (raw)
In-Reply-To: <2dd723c8-6aed-857c-23f3-d0381fcb52c2@arm.com>

On Thu, Sep 30, 2021 at 11:00:06AM +0100, Steven Price wrote:
>On 29/09/2021 19:33, Lucas De Marchi wrote:
>> Like the IS_ENABLED() counterpart, we can make IS_CONFIG_NONZERO() to
>> return the right thing when the config is not defined rather than a
>> build error, with the limitation that it can't be used on preprocessor
>> context.
>>
>> The trick here is that macro names can't start with a number or dash, so
>> we stringify the argument and check that the first char is a number != 0
>> (or starting with a dash to cover negative numbers). Except for -O0
>> builds the strings are all eliminated.
>>
>> Taking CONFIG_DRM_I915_REQUEST_TIMEOUT in
>> drivers/gpu/drm/i915/gem/i915_gem_context.c as example, we have the
>> following output of the preprocessor:
>>
>> old:
>>  if (((20000) != 0) &&
>> new:
>>  if (( ("20000"[0] > '0' && "20000"[0] < '9') || "20000"[0] == '-' ) &&
>>
>> New one looks worse, but is also eliminated from the object:
>>
>> $ size drivers/gpu/drm/i915/gem/i915_gem_context.o.*
>>    text    data     bss     dec     hex filename
>>   52021    1070     232   53323    d04b drivers/gpu/drm/i915/gem/i915_gem_context.o.new
>>   52021    1070     232   53323    d04b drivers/gpu/drm/i915/gem/i915_gem_context.o.old
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_utils.h | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
>> index 02bbfa4d68d3..436ce612c46a 100644
>> --- a/drivers/gpu/drm/i915/i915_utils.h
>> +++ b/drivers/gpu/drm/i915/i915_utils.h
>> @@ -28,6 +28,7 @@
>>  #include <linux/list.h>
>>  #include <linux/overflow.h>
>>  #include <linux/sched.h>
>> +#include <linux/stringify.h>
>>  #include <linux/types.h>
>>  #include <linux/workqueue.h>
>>
>> @@ -469,6 +470,9 @@ static inline bool timer_expired(const struct timer_list *t)
>>   *
>>   * Returns 0 if @config is 0, 1 if set to any value.
>>   */
>> -#define IS_CONFIG_NONZERO(config) ((config) != 0)
>> +#define IS_CONFIG_NONZERO(config) (						\
>> +	(__stringify_1(config)[0] > '0' && __stringify_1(config)[0] < '9') ||	\
>
>Shouldn't this be "<= '9'". Otherwise numbers starting with a 9 are not
>"non zero".

yes! thanks for catching it. However from the other discussion it seems
we can either

a) just remove the macro, or
b) use the simpler version that doesn't cover undefined values

I will investigate those options.

Lucas De Marchi

  reply	other threads:[~2021-09-30 15:43 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-29 18:33 [PATCH v2 0/3] Rename IS_ACTIVE() and move to kconfig.h Lucas De Marchi
2021-09-29 18:33 ` [PATCH v2 1/3] drm/i915: rename IS_ACTIVE Lucas De Marchi
2021-09-30 10:46   ` Jani Nikula
2021-09-30 15:47     ` Lucas De Marchi
2021-09-29 18:33 ` [PATCH v2 2/3] drm/i915/utils: do not depend on config being defined Lucas De Marchi
2021-09-29 21:08   ` Andrzej Hajda
2021-09-29 22:54     ` Lucas De Marchi
2021-09-30  7:01       ` Andrzej Hajda
2021-09-30 10:00   ` Steven Price
2021-09-30 15:43     ` Lucas De Marchi [this message]
2021-09-29 18:33 ` [PATCH v2 3/3] Move IS_CONFIG_NONZERO() to kconfig.h Lucas De Marchi
2021-09-30 14:01   ` Masahiro Yamada
2021-09-30 15:55     ` Lucas De Marchi
2021-10-01  2:33       ` Masahiro Yamada

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210930154315.xb43gowfhmxucsm4@ldmartin-desk2 \
    --to=lucas.demarchi@intel.com \
    --cc=daniel.vetter@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masahiroy@kernel.org \
    --cc=steven.price@arm.com \
    --subject='Re: [PATCH v2 2/3] drm/i915/utils: do not depend on config being defined' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).