On Thu, Nov 2, 2023 at 1:12 PM Nathan Chancellor <nathan@xxxxxxxxxx> wrote:
>
> 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?
I think replacing the clang check entirely. A similar issue was just
reported on different GCC versions:
https://lists.freedesktop.org/archives/amd-gfx/2023-November/100725.html
Alex
>
> 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
> >
