On Thu, Nov 02, 2023 at 12:59:00PM -0400, Hamza Mahfooz wrote:
> On 11/2/23 12:24, Nathan Chancellor wrote:
> > When building ARCH=x86_64 allmodconfig with clang, which have sanitizers
> > enabled, there is a warning about a large stack frame.
> >
> > drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:6265:13: error: stack frame size (2520) exceeds limit (2048) in 'dml_prefetch_check' [-Werror,-Wframe-larger-than]
> > 6265 | static void dml_prefetch_check(struct display_mode_lib_st *mode_lib)
> > | ^
> > 1 error generated.
> >
> > Notably, GCC 13.2.0 does not do too much of a better job, as it is right
> > at the current limit of 2048:
> >
> > drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c: In function 'dml_prefetch_check':
> > drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:6705:1: error: the frame size of 2048 bytes is larger than 1800 bytes [-Werror=frame-larger-than=]
> > 6705 | }
> > | ^
> >
> > In the past, these warnings have been avoided by reducing the number of
> > parameters to various functions so that not as many arguments need to be
> > passed on the stack. However, these patches take a good amount of effort
> > to write despite being mechanical due to code structure and complexity
> > and they are never carried forward to new generations of the code so
> > that effort has to be expended every new hardware generation, which
> > becomes harder to justify as time goes on.
> >
> > There is some effort to improve clang's code generation but that may
> > take some time between code review, shifting priorities, and release
> > cycles. To avoid having a noticeable or lengthy breakage in
> > all{mod,yes}config, which are easy testing targets that have -Werror
> > enabled, increase the limit for clang by 50% so that cases of extremely
> > poor code generation can still be caught while not breaking the majority
> > of builds. When clang's code generation improves, the limit increase can
> > be restricted to older clang versions.
> >
> > Signed-off-by: Nathan Chancellor <nathan@xxxxxxxxxx>
> > ---
> > If there is another DRM pull before 6.7-rc1, it would be much
> > appreciated if this could make that so that other trees are not
> > potentially broken by this. If not, no worries, as it was my fault for
> > not sending this sooner.
> > ---
> > drivers/gpu/drm/amd/display/dc/dml2/Makefile | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dml2/Makefile b/drivers/gpu/drm/amd/display/dc/dml2/Makefile
> > index 70ae5eba624e..dff8237c0999 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dml2/Makefile
> > +++ b/drivers/gpu/drm/amd/display/dc/dml2/Makefile
> > @@ -60,7 +60,7 @@ endif
> > endif
> > ifneq ($(CONFIG_FRAME_WARN),0)
> > -frame_warn_flag := -Wframe-larger-than=2048
> > +frame_warn_flag := -Wframe-larger-than=$(if $(CONFIG_CC_IS_CLANG),3072,2048)
>
> I would prefer checking for `CONFIG_KASAN || CONFIG_KCSAN` instead
> since the stack usage shouldn't change much if both of those are disabled.
So something like this? Or were you talking about replacing the clang
check entirely with the KASAN/KCSAN check?
diff --git a/drivers/gpu/drm/amd/display/dc/dml2/Makefile b/drivers/gpu/drm/amd/display/dc/dml2/Makefile
index 70ae5eba624e..0fc1b13295eb 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dml2/Makefile
@@ -60,8 +60,12 @@ endif
endif
ifneq ($(CONFIG_FRAME_WARN),0)
+ifeq ($(CONFIG_CC_IS_CLANG)$(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),yy)
+frame_warn_flag := -Wframe-larger-than=3072
+else
frame_warn_flag := -Wframe-larger-than=2048
endif
+endif
CFLAGS_$(AMDDALPATH)/dc/dml2/display_mode_core.o := $(dml2_ccflags) $(frame_warn_flag)
CFLAGS_$(AMDDALPATH)/dc/dml2/display_mode_util.o := $(dml2_ccflags)
> > endif
> > CFLAGS_$(AMDDALPATH)/dc/dml2/display_mode_core.o := $(dml2_ccflags) $(frame_warn_flag)
> >
> > ---
> > base-commit: 21e80f3841c01aeaf32d7aee7bbc87b3db1aa0c6
> > change-id: 20231102-amdgpu-dml2-increase-frame-size-warning-for-clang-c93bd2d6a871
> >
> > Best regards,
> --
> Hamza
>
