| Message ID | 20240829155950.1141978-1-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm/i915/fence: A couple of build fixes | expand |
On Thu, 29 Aug 2024, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > With CONFIG_WERROR=y and `make W=1` build fails on my x86_64 machine. > This is due to some unused functions. Hence these quick fixes. Since when have we been getting the warnings for static inlines? BR, Jani. > > Andy Shevchenko (2): > drm/i915/fence: Mark debug_fence_init_onstack() with __maybe_unused > drm/i915/fence: Mark debug_fence_free() with __maybe_unused > > drivers/gpu/drm/i915/i915_sw_fence.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-)
On Thu, Aug 29, 2024 at 07:38:08PM +0300, Jani Nikula wrote: > On Thu, 29 Aug 2024, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > With CONFIG_WERROR=y and `make W=1` build fails on my x86_64 machine. > > This is due to some unused functions. Hence these quick fixes. > > Since when have we been getting the warnings for static inlines? Do you want to see any additional information of my building environment? Debian clang version 18.1.8 (9)
On Thu, Aug 29, 2024 at 07:53:25PM +0300, Andy Shevchenko wrote: > On Thu, Aug 29, 2024 at 07:38:08PM +0300, Jani Nikula wrote: > > On Thu, 29 Aug 2024, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > > With CONFIG_WERROR=y and `make W=1` build fails on my x86_64 machine. > > > This is due to some unused functions. Hence these quick fixes. > > > > Since when have we been getting the warnings for static inlines? > > Do you want to see any additional information of my building environment? > > Debian clang version 18.1.8 (9) FWIW, clang-16 also behaves in the same way, Cc'ed to CLANG maintainers.
On Thu, Aug 29, 2024 at 09:10:41PM +0300, Andy Shevchenko wrote: > On Thu, Aug 29, 2024 at 07:53:25PM +0300, Andy Shevchenko wrote: > > On Thu, Aug 29, 2024 at 07:38:08PM +0300, Jani Nikula wrote: > > > On Thu, 29 Aug 2024, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > > > With CONFIG_WERROR=y and `make W=1` build fails on my x86_64 machine. > > > > This is due to some unused functions. Hence these quick fixes. > > > > > > Since when have we been getting the warnings for static inlines? Since commit 6863f5643dd7 ("kbuild: allow Clang to find unused static inline functions for W=1 build"). clang warns about unused static inline functions in .c files, unlike GCC (they both do not warn for functions coming from .h files). This difference is worked around for the normal build by adding '__maybe_unused' to the definition of 'inline' but Masahiro wanted to disable it for W=1 to allow this difference to find unused/dead code. There have not been too many complaints as far as I am aware but I can see how it is surprising. Cheers, Nathan
On Thu, 29 Aug 2024, Nathan Chancellor <nathan@kernel.org> wrote: > On Thu, Aug 29, 2024 at 09:10:41PM +0300, Andy Shevchenko wrote: >> On Thu, Aug 29, 2024 at 07:53:25PM +0300, Andy Shevchenko wrote: >> > On Thu, Aug 29, 2024 at 07:38:08PM +0300, Jani Nikula wrote: >> > > On Thu, 29 Aug 2024, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: >> > > > With CONFIG_WERROR=y and `make W=1` build fails on my x86_64 machine. >> > > > This is due to some unused functions. Hence these quick fixes. >> > > >> > > Since when have we been getting the warnings for static inlines? > > Since commit 6863f5643dd7 ("kbuild: allow Clang to find unused static > inline functions for W=1 build"). clang warns about unused static inline > functions in .c files, unlike GCC (they both do not warn for functions > coming from .h files). This difference is worked around for the normal > build by adding '__maybe_unused' to the definition of 'inline' but > Masahiro wanted to disable it for W=1 to allow this difference to find > unused/dead code. There have not been too many complaints as far as I am > aware but I can see how it is surprising. Heh, I was just going to reply citing the same commit. I occasionally build with clang myself, and we do enable most W=1 by default in the drm subsystem, so I was wondering why I hadn't hit this. The crucial difference is that we lack -DKBUILD_EXTRA_WARN1 which W=1 adds. I see there's no subdir-cppflags-y, but I don't see any harm in us adding -Wundef and -DKBUILD_EXTRA_WARN1 to subdir-ccflags-y. After we fix the fallout, of course. Do you? I don't much like the __maybe_unused stuff, but I guess it's fine as a stopgap measure, and then we can grep for that when running out of things to do. :p The TL;DR is, Reviewed-by: Jani Nikula <jani.nikula@intel.com> on the series. BR, Jani.
On Thu, Aug 29, 2024 at 09:37:34PM +0300, Jani Nikula wrote: > On Thu, 29 Aug 2024, Nathan Chancellor <nathan@kernel.org> wrote: > > Since commit 6863f5643dd7 ("kbuild: allow Clang to find unused static > > inline functions for W=1 build"). clang warns about unused static inline > > functions in .c files, unlike GCC (they both do not warn for functions > > coming from .h files). This difference is worked around for the normal > > build by adding '__maybe_unused' to the definition of 'inline' but > > Masahiro wanted to disable it for W=1 to allow this difference to find > > unused/dead code. There have not been too many complaints as far as I am > > aware but I can see how it is surprising. > > Heh, I was just going to reply citing the same commit. > > I occasionally build with clang myself, and we do enable most W=1 by > default in the drm subsystem, so I was wondering why I hadn't hit > this. The crucial difference is that we lack -DKBUILD_EXTRA_WARN1 which > W=1 adds. > > I see there's no subdir-cppflags-y, but I don't see any harm in us > adding -Wundef and -DKBUILD_EXTRA_WARN1 to subdir-ccflags-y. After we > fix the fallout, of course. Do you? No, that seems entirely reasonable when your goal is to enable W=1 for your subsystem. > I don't much like the __maybe_unused stuff, but I guess it's fine as a > stopgap measure, and then we can grep for that when running out of > things to do. :p Perhaps worth a TODO or something? Maybe a kernel newbie could work on that at some point if it is not high enough priority. Cheers, Nathan
On Thu, 29 Aug 2024, Jani Nikula <jani.nikula@linux.intel.com> wrote: > The TL;DR is, > > Reviewed-by: Jani Nikula <jani.nikula@intel.com> > > on the series. Both pushed to drm-intel-next, thanks for the patches and discussion. I amended the commit message about clang, config options and commit 6863f5643dd7 ("kbuild: allow Clang to find unused static inline functions for W=1 build") while pushing. BR, Jani.
On Mon, Sep 02, 2024 at 11:27:57AM +0300, Jani Nikula wrote: > On Thu, 29 Aug 2024, Jani Nikula <jani.nikula@linux.intel.com> wrote: > > The TL;DR is, > > > > Reviewed-by: Jani Nikula <jani.nikula@intel.com> > > > > on the series. > > Both pushed to drm-intel-next, thanks for the patches and discussion. > > I amended the commit message about clang, config options and commit > 6863f5643dd7 ("kbuild: allow Clang to find unused static inline > functions for W=1 build") while pushing. It all makes sense. Thank you!