[PATCH] Enable '-Werror' by default for all kernel builds

[Linus Torvalds]

On Tue, Sep 7, 2021 at 10:10 AM Linus Torvalds
<torvalds at linux-foundation.org> wrote:
>
> Do I know why? No. I do note that that code is disgusting.
>
> It's passing one of those structs around by value, for example. That's
> a 72-byte structure that is copied on the stack due to stupid calling
> conventions. Maybe clang generates a few extra temporaries for it as
> part of the function call stack setup? Who knows..

Ooh, yes.

This attached patch is crap - it converts the helper functions to use
const pointers instead of passing the whole structure, but it then
only converts that one file that *uses* them.

So the end result will not compile in general, but you can do

    make drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/display_rq_dlg_calc_30.o

and it compiles for me.

And while gcc doesn't care that much - it will apparently either
generate the argument stack every call - clang cares deeply.

The nasty 720-byte stack frame that clang generates turns into just a
320-byte one, and code generation in general looks a *lot* better.

Now, as mentioned, this patch is broken and incomplete. But I really
think the AMD GPU people need to do this. It makes those functions go
from practically unusable to not horribly disgusting.

So Harry/Leo/Alex/Christian and amd-gfx list - can you look into
making this ugly "make one file compile better" patch actually work
properly?

It *looks* lto me ike that code was perhaps written for a C++ compiler
and the helpers have been written as a "pass by reference", and the
arguments used to be

     const display_data_rq_misc_params_st& rq_misc_param

and then the compiler will pass the argument as a pointer. And then it
was converted to C, and the "pass by reference" in the function
declaration was turned into "pass by value", to avoid changing "." to
"->" in the use.

But a '&arg' thing in C++ really is a '*arg' pointer in C, and should
have been done as that.

Of course, it's also possible that that code was simply written by
somebody who didn't understand just *how* horrible it is to pass
structures bigger than a word or two by value.

Do we have a compiler warning for passing big structures by value?

                   Linus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.diff
Type: text/x-patch
Size: 17439 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20210907/6baf4df7/attachment-0001.bin>