From: Ard Biesheuvel <ardb@kernel.org> To: Paul Cercueil <paul@crapouillou.net> Cc: Linus Walleij <linus.walleij@linaro.org>, Arnd Bergmann <arnd@kernel.org>, "open list:MIPS" <linux-mips@vger.kernel.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, od@zcrc.me, Linux ARM <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH 1/2] if_enabled.h: Add IF_ENABLED_OR_ELSE() and IF_ENABLED() macros Date: Wed, 9 Dec 2020 10:38:51 +0100 [thread overview] Message-ID: <CAMj1kXGXsTTtPLX0HM3_HYRBTJ8q+w09xgXs9BXMSe8GLtk2YQ@mail.gmail.com> (raw) In-Reply-To: <20201208164821.2686082-1-paul@crapouillou.net> On Tue, 8 Dec 2020 at 17:49, Paul Cercueil <paul@crapouillou.net> wrote: > > Introduce a new header <linux/if_enabled.h>, that brings two new macros: > IF_ENABLED_OR_ELSE() and IF_ENABLED(). > > IF_ENABLED_OR_ELSE(CONFIG_FOO, a, b) evaluates to (a) if CONFIG_FOO is set > to 'y' or 'm', (b) otherwise. It is used internally to define the > IF_ENABLED() macro. The (a) and (b) arguments must be of the same type. > > IF_ENABLED(CONFIG_FOO, ptr) evaluates to (ptr) if CONFIG_FOO is set to 'y' > or 'm', NULL otherwise. The (ptr) argument must be a pointer. > > The IF_ENABLED() macro can be very useful to help GCC drop dead code. > > For instance, consider the following: > > #ifdef CONFIG_FOO_SUSPEND > static int foo_suspend(struct device *dev) > { > ... > } > #endif > > static struct pm_ops foo_ops = { > #ifdef CONFIG_FOO_SUSPEND > .suspend = foo_suspend, > #endif > }; > > While this works, the foo_suspend() macro is compiled conditionally, > only when CONFIG_FOO_SUSPEND is set. This is problematic, as there could > be a build bug in this function, we wouldn't have a way to know unless > the config option is set. > > An alternative is to declare foo_suspend() always, but mark it as maybe > unused: > > static int __maybe_unused foo_suspend(struct device *dev) > { > ... > } > > static struct pm_ops foo_ops = { > #ifdef CONFIG_FOO_SUSPEND > .suspend = foo_suspend, > #endif > }; > > Again, this works, but the __maybe_unused attribute is required to > instruct the compiler that the function may not be referenced anywhere, > and is safe to remove without making a fuss about it. This makes the > programmer responsible for tagging the functions that can be > garbage-collected. > > With this patch, it is now possible to write the following: > > static int foo_suspend(struct device *dev) > { > ... > } > > static struct pm_ops foo_ops = { > .suspend = IF_ENABLED(CONFIG_FOO_SUSPEND, foo_suspend), > }; > > The foo_suspend() function will now be automatically dropped by the > compiler, and it does not require any specific attribute. > > Signed-off-by: Paul Cercueil <paul@crapouillou.net> Hi Paul, I understand the issue you are trying to address here, but please note that there are many cases where the struct member in question will not even be declared if the associated CONFIG option is not set, in which case this will cause a compile error. > --- > include/linux/if_enabled.h | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > create mode 100644 include/linux/if_enabled.h > > diff --git a/include/linux/if_enabled.h b/include/linux/if_enabled.h > new file mode 100644 > index 000000000000..8189d1402340 > --- /dev/null > +++ b/include/linux/if_enabled.h > @@ -0,0 +1,22 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef __LINUX_IF_ENABLED_H > +#define __LINUX_IF_ENABLED_H > + > +#include <linux/build_bug.h> > +#include <linux/compiler_types.h> > +#include <linux/kconfig.h> > + > +/* > + * IF_ENABLED_OR_ELSE(CONFIG_FOO, a, b) evaluates to (a) if CONFIG_FOO is set > + * to 'y' or 'm', (b) otherwise. > + */ > +#define IF_ENABLED_OR_ELSE(option, a, b) \ > + (BUILD_BUG_ON_ZERO(__same_type((a), (b))) || IS_ENABLED(option) ? (a) : (b)) > + > +/* > + * IF_ENABLED(CONFIG_FOO, ptr) evaluates to (ptr) if CONFIG_FOO is set to 'y' > + * or 'm', NULL otherwise. > + */ > +#define IF_ENABLED(option, ptr) IF_ENABLED_OR_ELSE(option, ptr, NULL) > + > +#endif /* __LINUX_IF_ENABLED_H */ > -- > 2.29.2 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Ard Biesheuvel <ardb@kernel.org> To: Paul Cercueil <paul@crapouillou.net> Cc: Arnd Bergmann <arnd@kernel.org>, Linus Walleij <linus.walleij@linaro.org>, "open list:MIPS" <linux-mips@vger.kernel.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, od@zcrc.me, Linux ARM <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH 1/2] if_enabled.h: Add IF_ENABLED_OR_ELSE() and IF_ENABLED() macros Date: Wed, 9 Dec 2020 10:38:51 +0100 [thread overview] Message-ID: <CAMj1kXGXsTTtPLX0HM3_HYRBTJ8q+w09xgXs9BXMSe8GLtk2YQ@mail.gmail.com> (raw) In-Reply-To: <20201208164821.2686082-1-paul@crapouillou.net> On Tue, 8 Dec 2020 at 17:49, Paul Cercueil <paul@crapouillou.net> wrote: > > Introduce a new header <linux/if_enabled.h>, that brings two new macros: > IF_ENABLED_OR_ELSE() and IF_ENABLED(). > > IF_ENABLED_OR_ELSE(CONFIG_FOO, a, b) evaluates to (a) if CONFIG_FOO is set > to 'y' or 'm', (b) otherwise. It is used internally to define the > IF_ENABLED() macro. The (a) and (b) arguments must be of the same type. > > IF_ENABLED(CONFIG_FOO, ptr) evaluates to (ptr) if CONFIG_FOO is set to 'y' > or 'm', NULL otherwise. The (ptr) argument must be a pointer. > > The IF_ENABLED() macro can be very useful to help GCC drop dead code. > > For instance, consider the following: > > #ifdef CONFIG_FOO_SUSPEND > static int foo_suspend(struct device *dev) > { > ... > } > #endif > > static struct pm_ops foo_ops = { > #ifdef CONFIG_FOO_SUSPEND > .suspend = foo_suspend, > #endif > }; > > While this works, the foo_suspend() macro is compiled conditionally, > only when CONFIG_FOO_SUSPEND is set. This is problematic, as there could > be a build bug in this function, we wouldn't have a way to know unless > the config option is set. > > An alternative is to declare foo_suspend() always, but mark it as maybe > unused: > > static int __maybe_unused foo_suspend(struct device *dev) > { > ... > } > > static struct pm_ops foo_ops = { > #ifdef CONFIG_FOO_SUSPEND > .suspend = foo_suspend, > #endif > }; > > Again, this works, but the __maybe_unused attribute is required to > instruct the compiler that the function may not be referenced anywhere, > and is safe to remove without making a fuss about it. This makes the > programmer responsible for tagging the functions that can be > garbage-collected. > > With this patch, it is now possible to write the following: > > static int foo_suspend(struct device *dev) > { > ... > } > > static struct pm_ops foo_ops = { > .suspend = IF_ENABLED(CONFIG_FOO_SUSPEND, foo_suspend), > }; > > The foo_suspend() function will now be automatically dropped by the > compiler, and it does not require any specific attribute. > > Signed-off-by: Paul Cercueil <paul@crapouillou.net> Hi Paul, I understand the issue you are trying to address here, but please note that there are many cases where the struct member in question will not even be declared if the associated CONFIG option is not set, in which case this will cause a compile error. > --- > include/linux/if_enabled.h | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > create mode 100644 include/linux/if_enabled.h > > diff --git a/include/linux/if_enabled.h b/include/linux/if_enabled.h > new file mode 100644 > index 000000000000..8189d1402340 > --- /dev/null > +++ b/include/linux/if_enabled.h > @@ -0,0 +1,22 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef __LINUX_IF_ENABLED_H > +#define __LINUX_IF_ENABLED_H > + > +#include <linux/build_bug.h> > +#include <linux/compiler_types.h> > +#include <linux/kconfig.h> > + > +/* > + * IF_ENABLED_OR_ELSE(CONFIG_FOO, a, b) evaluates to (a) if CONFIG_FOO is set > + * to 'y' or 'm', (b) otherwise. > + */ > +#define IF_ENABLED_OR_ELSE(option, a, b) \ > + (BUILD_BUG_ON_ZERO(__same_type((a), (b))) || IS_ENABLED(option) ? (a) : (b)) > + > +/* > + * IF_ENABLED(CONFIG_FOO, ptr) evaluates to (ptr) if CONFIG_FOO is set to 'y' > + * or 'm', NULL otherwise. > + */ > +#define IF_ENABLED(option, ptr) IF_ENABLED_OR_ELSE(option, ptr, NULL) > + > +#endif /* __LINUX_IF_ENABLED_H */ > -- > 2.29.2 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-12-09 9:39 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-12-08 16:48 [PATCH 1/2] if_enabled.h: Add IF_ENABLED_OR_ELSE() and IF_ENABLED() macros Paul Cercueil 2020-12-08 16:48 ` Paul Cercueil 2020-12-08 16:48 ` [PATCH 2/2] pinctrl: ingenic: Only support SoCs enabled in config Paul Cercueil 2020-12-08 16:48 ` Paul Cercueil 2020-12-09 10:13 ` Daniel Palmer 2020-12-09 10:13 ` Daniel Palmer 2020-12-09 11:04 ` Arnd Bergmann 2020-12-09 11:04 ` Arnd Bergmann 2020-12-08 17:39 ` [PATCH 1/2] if_enabled.h: Add IF_ENABLED_OR_ELSE() and IF_ENABLED() macros Randy Dunlap 2020-12-08 17:39 ` Randy Dunlap 2020-12-08 19:00 ` Paul Cercueil 2020-12-08 19:00 ` Paul Cercueil 2020-12-09 8:59 ` Linus Walleij 2020-12-09 8:59 ` Linus Walleij 2020-12-09 11:31 ` Paul Cercueil 2020-12-09 11:31 ` Paul Cercueil 2020-12-09 9:38 ` Ard Biesheuvel [this message] 2020-12-09 9:38 ` Ard Biesheuvel 2020-12-09 11:27 ` Paul Cercueil 2020-12-09 11:27 ` Paul Cercueil
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=CAMj1kXGXsTTtPLX0HM3_HYRBTJ8q+w09xgXs9BXMSe8GLtk2YQ@mail.gmail.com \ --to=ardb@kernel.org \ --cc=arnd@kernel.org \ --cc=linus.walleij@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mips@vger.kernel.org \ --cc=od@zcrc.me \ --cc=paul@crapouillou.net \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.