[PATCH] drm/amd/display: Increase frame warning limit for clang in dml2

[Hamza Mahfooz]

On 11/2/23 13:12, Nathan Chancellor 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 at kernel.org>
>>> ---
>>> 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 for the time being replacing the clang check with a KASAN/KCSAN
check would make more sense. Considering that, the allmodconfig for older
versions of gcc is also broken (see [1]).

1. 
https://lore.kernel.org/amd-gfx/CADVatmO9NCs=ryNg72HNzMDpqg862gpGnnFhQ4uwTpEkjOkCLw@mail.gmail.com/

> 
> 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
>>
-- 
Hamza