On Tue, Oct 10, 2023 at 1:50 AM Jani Nikula <jani.nikula@xxxxxxxxx> wrote: > > On Tue, 10 Oct 2023, Jani Nikula <jani.nikula@xxxxxxxxx> wrote: > > On Mon, 09 Oct 2023, Nathan Chancellor <nathan@xxxxxxxxxx> wrote: > >> On Sun, Oct 08, 2023 at 12:28:46AM +0900, Masahiro Yamada wrote: > >>> On Fri, Oct 6, 2023 at 9:35 PM Jani Nikula <jani.nikula@xxxxxxxxx> wrote: > >>> > > >>> > The kernel top level Makefile, and recently scripts/Makefile.extrawarn, > >>> > have included -Wall, and the disables -Wno-format-security and > >>> > $(call cc-disable-warning,frame-address,) for a very long time. They're > >>> > redundant in our local subdir-ccflags-y and can be dropped. > >>> > > >>> > Cc: Arnd Bergmann <arnd@xxxxxxxx> > >>> > Cc: Nick Desaulniers <ndesaulniers@xxxxxxxxxx> > >>> > Cc: Nathan Chancellor <nathan@xxxxxxxxxx> > >>> > Cc: Masahiro Yamada <masahiroy@xxxxxxxxxx> > >>> > Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> > >>> > >>> > >>> I made a similar suggestion in the past > >>> https://lore.kernel.org/dri-devel/20190515043753.9853-1-yamada.masahiro@xxxxxxxxxxxxx/ > >>> > >>> So, I am glad that Intel has decided to de-duplicate the flags. > >>> > >>> > >>> > >>> I think you can drop more flags. > >>> > >>> For example, > >>> > >>> subdir-ccflags-y += -Wno-sign-compare > >>> > >>> > >>> It is set by scripts/Makefile.extrawarn > >>> unless W=3 is passed. > >>> > >>> > >>> If W=3 is set by a user, -Wsign-compare should be warned > >>> as it is the user's request. > >>> > >>> > >>> drivers/gpu/drm/i915/Makefile negates W=3. > >>> There is no good reason to do so. > >>> > >>> > >>> Same applied to > >>> > >>> > >>> subdir-ccflags-y += -Wno-shift-negative-value > >> > >> As I point out in my review of the second patch [1], I am not sure these > >> should be dropped because -Wextra turns these warnings back on, at least > >> for clang according to this build report [2] and my own testing, so they > >> need to be disabled again. > > > > Yeah. The focus is on enabling W=1 warnings by default for i915. I get > > that the disables we have to add to achieve that also disable some W=2 > > and W=3 warnings. But taking all of that into account requires > > duplicating even more of Makefile.extrawarn (checking for warning > > levels, maintaining parity with the different levels, etc.). > > > > I guess we could check if KBUILD_EXTRA_WARN does not have any of 1, 2, > > or 3, but very few places outside of the build system look at > > KBUILD_EXTRA_WARN, so feels wrong. > > This is the simplest I could think of: > > # The following turn off the warnings enabled by -Wextra > ifeq ($(findstring 2, $(KBUILD_EXTRA_WARN)),) > KBUILD_CFLAGS += -Wno-missing-field-initializers > KBUILD_CFLAGS += -Wno-type-limits > KBUILD_CFLAGS += -Wno-shift-negative-value > endif > ifeq ($(findstring 3, $(KBUILD_EXTRA_WARN)),) > KBUILD_CFLAGS += -Wno-sign-compare > endif > > Masahiro, I'd like to get your feedback on which to choose, > unconditionally silencing the W=2/W=3 warnings for i915, or looking at > KBUILD_EXTRA_WARN. KBUILD_EXTRA_WARN looks better to me; otherwise they would be hidden forever (or nearly). Suffer some duplication, w/e. -- Thanks, ~Nick Desaulniers
